-
Notifications
You must be signed in to change notification settings - Fork 50
✨ Create Insights page #2488
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.
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
✨ Create Insights page #2488
Conversation
WalkthroughThis change introduces a new "Insights" feature to the application, including new data models, API types, routes, and UI components. The insights functionality parallels the existing issues feature, with dedicated pages, tables, filters, and detail views. Numerous components, hooks, and helpers were added or refactored to support insights, while some "issues" components were removed or updated to reflect the new structure. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Sidebar
participant Router
participant InsightsPage
participant InsightsAPI
participant InsightsTable
participant InsightDetailDrawer
User->>Sidebar: Clicks "Insights"
Sidebar->>Router: Navigates to /insights
Router->>InsightsPage: Renders InsightsPage
InsightsPage->>InsightsAPI: Fetch insights data
InsightsAPI-->>InsightsPage: Returns insights
InsightsPage->>InsightsTable: Renders table with insights
User->>InsightsTable: Clicks on insight row
InsightsTable->>InsightDetailDrawer: Opens detail drawer for insight
InsightDetailDrawer->>InsightsAPI: Fetches insight details
InsightsAPI-->>InsightDetailDrawer: Returns insight details
InsightDetailDrawer-->>User: Displays insight detail
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
client/src/app/pages/insights/helpers.ts (1)
1-274
: Significant code duplication detected.This entire file appears to be a near-exact duplicate of
client/src/app/pages/issues/helpers.ts
with minimal naming changes. While this may be intentional per the PR objectives (creating a "clone" of the Issues page), it creates significant maintenance burden.Consider extracting the common functionality into shared utilities to reduce duplication:
// shared/page-helpers.ts export const createPageHelpers = (config: { paths: { allTab: string; singleAppTab: string; singleAppSelected: string; allAffectedApplications: string; }; persistenceKeyPrefix: { main: string; affectedApps: string; }; fromParamsKey: string; filterGroups: { applicationInventory: string; main: string; }; }) => { // Return all the helper functions with the config applied }; // Then use it in both issues and insights helpers: export const insightsHelpers = createPageHelpers({ paths: { allTab: Paths.insightsAllTab, // ... other paths }, // ... other config });This would eliminate ~90% of the code duplication while maintaining the separate APIs.
🧹 Nitpick comments (8)
client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/incident-code-snip-viewer.css (1)
5-5
: Consider removing the !important declaration and using proper CSS specificity.The
!important
declaration suggests there might be specificity conflicts. It's better to increase specificity through proper selectors rather than using!important
, which makes styles harder to override and maintain.client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-all-incidents-table.tsx (1)
31-31
: Consider using insights-specific persistence key.The component uses
TablePersistenceKeyPrefix.issuesRemainingIncidents
but this table is used in both issues and insights contexts. Consider using the appropriate persistence key based on context or creating a shared key.This could be addressed by either:
- Passing the appropriate persistence key as a prop
- Using a context-agnostic key name
- Determining the context dynamically
- persistenceKeyPrefix: TablePersistenceKeyPrefix.issuesRemainingIncidents, + persistenceKeyPrefix: TablePersistenceKeyPrefix.insightsRemainingIncidents, // or make it configurableclient/src/app/pages/insights/affected-applications/affected-applications.tsx (1)
155-157
: Move inline styles to CSS or use PatternFly utilitiesConsider using PatternFly's utility classes or moving this to a CSS file for better maintainability.
Replace the inline style with a PatternFly utility class:
- <div - style={{ - backgroundColor: "var(--pf-v5-global--BackgroundColor--100)", - }} - > + <div className="pf-v5-u-background-color-100">client/src/app/pages/insights/insight-detail-drawer/insight-affected-files-table.tsx (1)
57-61
: Address the i18n TODO commentThe TODO comment indicates missing internationalization for "file".
Would you like me to help implement the proper i18n for this filter placeholder text or open an issue to track this task?
client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/file-all-incidents-table.tsx (1)
85-95
: Consider moving pagination to toolbarHaving pagination controls inside the table header is unconventional. Consider moving it to a toolbar above the table for better UX consistency.
client/src/app/pages/insights/insights-table.tsx (1)
173-184
: Consider extracting effort filtering logic.The commented-out effort filtering logic suggests uncertainty about the filtering approach. This indicates a potential area for future enhancement.
Consider creating a separate issue to determine the requirements for effort-based filtering and implement it if needed.
client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/incident-code-snip-viewer.tsx (1)
63-68
: Language detection could be more robust.The language detection relies on file extension mapping but doesn't handle edge cases well.
Consider adding fallback handling for unknown extensions:
const extension = incident.file.toLowerCase().split(".").slice(-1)[0]; const language = Object.keys(LANGUAGES_BY_FILE_EXTENSION).includes(extension) ? (LANGUAGES_BY_FILE_EXTENSION[ extension as keyof typeof LANGUAGES_BY_FILE_EXTENSION ] as Language) - : undefined; + : Language.plaintext; // Fallback to plaintext for unknown extensionsclient/src/app/queries/analysis.ts (1)
230-230
: Consider improving error handling beyond console logging.While the
onError
callbacks are valid in React Query v4, simply logging errors to the console may not provide adequate error handling for production use. Consider implementing proper error reporting or user notifications.Also applies to: 253-253, 276-276
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
client/public/locales/en/translation.json
(2 hunks)client/src/app/Constants.ts
(1 hunks)client/src/app/Paths.ts
(1 hunks)client/src/app/Routes.tsx
(3 hunks)client/src/app/api/models.ts
(2 hunks)client/src/app/api/rest/analysis.ts
(4 hunks)client/src/app/layout/SidebarApp/SidebarApp.tsx
(1 hunks)client/src/app/pages/insights/affected-applications/affected-applications.tsx
(1 hunks)client/src/app/pages/insights/affected-applications/index.ts
(1 hunks)client/src/app/pages/insights/affected-apps-link.tsx
(1 hunks)client/src/app/pages/insights/components/insight-description-and-links.tsx
(1 hunks)client/src/app/pages/insights/helpers.ts
(1 hunks)client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/file-all-incidents-table.css
(1 hunks)client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/file-all-incidents-table.tsx
(1 hunks)client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/file-incidents-detail-modal.tsx
(1 hunks)client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/incident-code-snip-viewer.css
(1 hunks)client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/incident-code-snip-viewer.tsx
(1 hunks)client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/index.ts
(1 hunks)client/src/app/pages/insights/insight-detail-drawer/index.ts
(1 hunks)client/src/app/pages/insights/insight-detail-drawer/insight-affected-files-table.tsx
(1 hunks)client/src/app/pages/insights/insight-detail-drawer/insight-detail-drawer.tsx
(1 hunks)client/src/app/pages/insights/insight-detail-drawer/path-display.tsx
(1 hunks)client/src/app/pages/insights/insights-page.tsx
(1 hunks)client/src/app/pages/insights/insights-table.tsx
(1 hunks)client/src/app/pages/issues/affected-apps-link.tsx
(1 hunks)client/src/app/pages/issues/components/issue-description-and-links.tsx
(1 hunks)client/src/app/pages/issues/helpers.ts
(5 hunks)client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-all-incidents-table.tsx
(3 hunks)client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-incidents-detail-modal.tsx
(2 hunks)client/src/app/pages/issues/issue-detail-drawer/issue-affected-files-table.tsx
(3 hunks)client/src/app/pages/issues/issues-table.tsx
(5 hunks)client/src/app/queries/analysis.ts
(13 hunks)client/src/app/utils/query-utils.ts
(1 hunks)
🧠 Learnings (16)
📓 Common learnings
Learnt from: sjd78
PR: konveyor/tackle2-ui#2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like `getIssue` to `getInsight` and parameters like `issueId` to `insightId` in the analysis API client functions.
client/src/app/Constants.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/insights/helpers.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/components/issue-description-and-links.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-all-incidents-table.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-incidents-detail-modal.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/issue-detail-drawer/issue-affected-files-table.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/Paths.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/issues-table.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/utils/query-utils.ts (1)
Learnt from: sjd78
PR: #2432
File: client/src/app/components/schema-defined-fields/utils.tsx:42-47
Timestamp: 2025-07-17T23:29:20.652Z
Learning: In the tackle2-ui codebase, when using yup.lazy for optional object validation, the correct TypeScript typing is ReturnType<typeof yup.lazy<yup.AnySchema>>
instead of passing the generic type parameter directly to yup.lazy, due to the specific version of yup being used.
client/public/locales/en/translation.json (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/helpers.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/api/rest/analysis.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/Routes.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/api/models.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/queries/analysis.ts (2)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
Learnt from: sjd78
PR: #2467
File: client/src/app/queries/generators.ts:20-20
Timestamp: 2025-07-21T06:01:43.078Z
Learning: The tackle2-ui codebase uses React Query v4 (@tanstack/react-query v4.22.0), where the onError callback in query configurations is still valid and supported. The deprecation of onError callbacks only applies to React Query v5, so suggestions about removing them are premature until the codebase is upgraded.
🧬 Code Graph Analysis (10)
client/src/app/layout/SidebarApp/SidebarApp.tsx (1)
client/src/app/Paths.ts (1)
DevPaths
(1-55)
client/src/app/pages/insights/affected-apps-link.tsx (2)
client/src/app/api/models.ts (1)
UiAnalysisReportInsight
(662-662)client/src/app/pages/insights/helpers.ts (2)
InsightsFilterValuesToCarry
(37-37)getAffectedAppsUrl
(146-174)
client/src/app/pages/issues/components/issue-description-and-links.tsx (1)
client/src/app/api/models.ts (1)
AnalysisInsightLink
(640-643)
client/src/app/pages/issues/affected-apps-link.tsx (1)
client/src/app/api/models.ts (1)
UiAnalysisReportInsight
(662-662)
client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-all-incidents-table.tsx (2)
client/src/app/api/models.ts (1)
AnalysisReportFile
(707-713)client/src/app/queries/analysis.ts (1)
useFetchIncidentsForInsight
(241-262)
client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-incidents-detail-modal.tsx (2)
client/src/app/api/models.ts (2)
AnalysisInsight
(614-628)AnalysisReportFile
(707-713)client/src/app/queries/analysis.ts (1)
useFetchIncidentsForInsight
(241-262)
client/src/app/pages/issues/issue-detail-drawer/issue-affected-files-table.tsx (1)
client/src/app/api/models.ts (2)
AnalysisInsight
(614-628)AnalysisReportFile
(707-713)
client/src/app/utils/query-utils.ts (2)
client/src/app/api/models.ts (1)
WithUiId
(17-17)client/src/app/Constants.ts (1)
UI_UNIQUE_ID
(27-27)
client/src/app/pages/issues/helpers.ts (2)
client/src/app/queries/tags.ts (1)
useFetchTagsWithTagItems
(65-110)client/src/app/api/models.ts (3)
UiAnalysisReportInsight
(662-662)UiAnalysisReportApplicationInsight
(680-681)AnalysisInsight
(614-628)
client/src/app/api/rest/analysis.ts (2)
client/src/app/api/rest.ts (3)
hub
(55-65)template
(54-54)getHubPaginatedResult
(130-142)client/src/app/api/models.ts (6)
AnalysisInsight
(614-628)AnalysisReportFile
(707-713)HubRequestParams
(31-41)AnalysisReportInsight
(648-660)AnalysisReportApplicationInsight
(665-678)AnalysisReportInsightApplication
(687-704)
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: sjd78
PR: konveyor/tackle2-ui#2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like `getIssue` to `getInsight` and parameters like `issueId` to `insightId` in the analysis API client functions.
client/src/app/Constants.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/insights/helpers.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/components/issue-description-and-links.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-all-incidents-table.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-incidents-detail-modal.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/issue-detail-drawer/issue-affected-files-table.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/Paths.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/issues-table.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/utils/query-utils.ts (1)
Learnt from: sjd78
PR: #2432
File: client/src/app/components/schema-defined-fields/utils.tsx:42-47
Timestamp: 2025-07-17T23:29:20.652Z
Learning: In the tackle2-ui codebase, when using yup.lazy for optional object validation, the correct TypeScript typing is ReturnType<typeof yup.lazy<yup.AnySchema>>
instead of passing the generic type parameter directly to yup.lazy, due to the specific version of yup being used.
client/public/locales/en/translation.json (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/helpers.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/api/rest/analysis.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/Routes.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/api/models.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/queries/analysis.ts (2)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
Learnt from: sjd78
PR: #2467
File: client/src/app/queries/generators.ts:20-20
Timestamp: 2025-07-21T06:01:43.078Z
Learning: The tackle2-ui codebase uses React Query v4 (@tanstack/react-query v4.22.0), where the onError callback in query configurations is still valid and supported. The deprecation of onError callbacks only applies to React Query v5, so suggestions about removing them are premature until the codebase is upgraded.
🧬 Code Graph Analysis (10)
client/src/app/layout/SidebarApp/SidebarApp.tsx (1)
client/src/app/Paths.ts (1)
DevPaths
(1-55)
client/src/app/pages/insights/affected-apps-link.tsx (2)
client/src/app/api/models.ts (1)
UiAnalysisReportInsight
(662-662)client/src/app/pages/insights/helpers.ts (2)
InsightsFilterValuesToCarry
(37-37)getAffectedAppsUrl
(146-174)
client/src/app/pages/issues/components/issue-description-and-links.tsx (1)
client/src/app/api/models.ts (1)
AnalysisInsightLink
(640-643)
client/src/app/pages/issues/affected-apps-link.tsx (1)
client/src/app/api/models.ts (1)
UiAnalysisReportInsight
(662-662)
client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-all-incidents-table.tsx (2)
client/src/app/api/models.ts (1)
AnalysisReportFile
(707-713)client/src/app/queries/analysis.ts (1)
useFetchIncidentsForInsight
(241-262)
client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-incidents-detail-modal.tsx (2)
client/src/app/api/models.ts (2)
AnalysisInsight
(614-628)AnalysisReportFile
(707-713)client/src/app/queries/analysis.ts (1)
useFetchIncidentsForInsight
(241-262)
client/src/app/pages/issues/issue-detail-drawer/issue-affected-files-table.tsx (1)
client/src/app/api/models.ts (2)
AnalysisInsight
(614-628)AnalysisReportFile
(707-713)
client/src/app/utils/query-utils.ts (2)
client/src/app/api/models.ts (1)
WithUiId
(17-17)client/src/app/Constants.ts (1)
UI_UNIQUE_ID
(27-27)
client/src/app/pages/issues/helpers.ts (2)
client/src/app/queries/tags.ts (1)
useFetchTagsWithTagItems
(65-110)client/src/app/api/models.ts (3)
UiAnalysisReportInsight
(662-662)UiAnalysisReportApplicationInsight
(680-681)AnalysisInsight
(614-628)
client/src/app/api/rest/analysis.ts (2)
client/src/app/api/rest.ts (3)
hub
(55-65)template
(54-54)getHubPaginatedResult
(130-142)client/src/app/api/models.ts (6)
AnalysisInsight
(614-628)AnalysisReportFile
(707-713)HubRequestParams
(31-41)AnalysisReportInsight
(648-660)AnalysisReportApplicationInsight
(665-678)AnalysisReportInsightApplication
(687-704)
⏰ 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). (1)
- GitHub Check: unit-test
🔇 Additional comments (56)
client/src/app/utils/query-utils.ts (1)
13-30
: LGTM! Enhanced type safety and flexibility.The updated generic type parameters
<F, T extends F = F>
provide better type safety by clearly separating the base API data type (F
) from the UI-enhanced type (T
). The explicit type assertionas WithUiId<T>
ensures proper typing of the mapped result, which aligns well with theWithUiId<T>
type introduced for the insights feature.This change maintains backward compatibility while enabling more flexible usage patterns for the insights/issues separation.
client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/file-all-incidents-table.css (1)
1-4
: LGTM! Appropriate styling for empty table cells.The
.empty-cell
class provides appropriate visual styling with muted color and italic font to clearly indicate empty or placeholder content in the incidents table.client/src/app/layout/SidebarApp/SidebarApp.tsx (1)
134-138
: LGTM! Consistent navigation implementation.The insights navigation item follows the established pattern used by other sidebar items, with proper routing, translation support, and logical positioning between issues and dependencies.
client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/index.ts (1)
1-1
: LGTM! Standard barrel export pattern.The re-export follows established conventions and facilitates easier imports of the file incidents detail modal components.
client/src/app/pages/insights/insight-detail-drawer/index.ts (1)
1-1
: LGTM! Consistent barrel export pattern.The re-export maintains consistency with other component modules and simplifies imports for the insight detail drawer functionality.
client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/incident-code-snip-viewer.css (1)
6-12
: Well-designed gradient technique for icon background.The linear gradient approach to create a white background only behind the exclamation mark icon is clever and efficient. This technique provides precise control over the visual appearance without requiring additional DOM elements.
client/src/app/pages/insights/affected-applications/index.ts (1)
1-1
: Standard barrel export pattern implemented correctly.This follows the established convention for creating clean import paths and organizing component exports.
client/public/locales/en/translation.json (2)
280-280
: Proper localization support for sidebar navigation.The addition of the "insights" key in the sidebar section correctly supports the new insights navigation feature.
409-409
: Consistent terminology localization added.The "insights" entry in the terms section provides proper localization support for general usage throughout the application.
client/src/app/pages/issues/affected-apps-link.tsx (2)
5-5
: Type import updated correctly for insights refactoring.The import change from
AnalysisRuleReport
toUiAnalysisReportInsight
aligns with the broader refactoring effort to support the new insights data model.
9-9
: Interface prop type updated consistently.The prop type change maintains compatibility while adopting the new
UiAnalysisReportInsight
type that includes UI identifiers.client/src/app/pages/issues/components/issue-description-and-links.tsx (2)
6-6
: Import updated for insights refactoring consistency.The change from
AnalysisIssueLink
toAnalysisInsightLink
aligns with the broader issues-to-insights refactoring effort tracked in issue #2445.
12-12
: Prop type updated to maintain consistency.The type change from
AnalysisIssueLink[]
toAnalysisInsightLink[]
ensures consistent typing across components while maintaining the same functionality.client/src/app/pages/issues/issue-detail-drawer/issue-affected-files-table.tsx (1)
11-11
: LGTM! Type updates are consistent with the insights refactoring.The type changes from
AnalysisFileReport
toAnalysisReportFile
andAnalysisIssue
toAnalysisInsight
align correctly with the new data model definitions. The component functionality remains unchanged while properly adopting the updated types.Also applies to: 30-30, 107-107
client/src/app/Constants.ts (1)
253-256
: LGTM! New insights persistence keys follow established patterns.The new
TablePersistenceKeyPrefix
enum values for insights functionality are well-designed:
- Follow the short, unique string pattern
- Mirror the existing issues structure
- Provide necessary persistence keys for the new insights tables
client/src/app/Paths.ts (1)
44-49
: LGTM! New insights paths mirror the established issues pattern.The new path constants for insights functionality are well-structured:
- Follow the same pattern as existing issues paths
- Provide complete route coverage (base, tabs, affected applications, single app)
- Maintain consistency in naming conventions
client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-all-incidents-table.tsx (1)
5-6
: LGTM! Type and hook updates are correctly implemented.The changes properly align with the insights refactoring:
AnalysisReportFile
type usage is correctuseFetchIncidentsForInsight
hook correctly usesfileReport.insightId
- Hook parameters and usage match the updated API signatures
Also applies to: 22-22, 47-48
client/src/app/pages/issues/issues-table.tsx (3)
68-69
: LGTM! Type updates correctly implement UI wrapper types.The type changes from
AnalysisIssueReport
/AnalysisRuleReport
toUiAnalysisReportApplicationInsight
/UiAnalysisReportInsight
are consistent with the refactoring to use UI wrapper types that include unique IDs for table management.Also applies to: 214-216, 240-240
420-427
: LGTM! Type assertions in render methods are correctly updated.The type assertions to
UiAnalysisReportApplicationInsight
in the onClick handlers and render logic properly handle the different report types based on the table mode (singleApp vs allIssues).Also applies to: 469-477
431-431
: LGTM! AffectedAppsLink prop types correctly updated.The
ruleReport
props are properly typed asUiAnalysisReportInsight
which aligns with the component's expected interface and the new data model.Also applies to: 482-482
client/src/app/pages/insights/insight-detail-drawer/path-display.tsx (1)
1-24
: LGTM - Clean path display utility with good line-breaking support.The component correctly normalizes path separators and implements graceful line breaking at slashes using
<wbr>
elements. The use ofreduce
to build the React node array is appropriate, and the key generation strategy is acceptable for this use case.client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-incidents-detail-modal.tsx (3)
20-21
: LGTM - Correct type alignment with insights refactoring.The import updates from
AnalysisIssue
/AnalysisFileReport
toAnalysisInsight
/AnalysisReportFile
align with the broader insights refactoring effort.
28-29
: LGTM - Props updated to use new insight types.The prop type changes are consistent with the updated data model and support the insights feature separation.
43-46
: LGTM - Hook updated to insights-specific variant.The change from
useFetchIncidents
touseFetchIncidentsForInsight
withfileReport.insightId
correctly aligns with the new data structure and API design.client/src/app/pages/insights/affected-apps-link.tsx (1)
1-34
: LGTM - Well-structured link component with clean URL generation.The component follows good React patterns with proper TypeScript interfaces and integrates cleanly with React Router. The conditional text display based on
showNumberOnly
provides good flexibility for different use cases.client/src/app/pages/insights/components/insight-description-and-links.tsx (1)
1-36
: LGTM - Clean component with proper markdown rendering and link handling.The component correctly handles optional props, uses ReactMarkdown with PatternFly components for consistent styling, and implements proper conditional rendering for the links section. The key usage (
link.url
) is appropriate for the link list.client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/file-incidents-detail-modal.tsx (1)
1-149
: LGTM - Well-implemented modal with good performance optimization.The component follows the same successful pattern as the issues version but correctly uses insights-specific types, hooks, and helpers. Key strengths:
- Proper state management for tab selection with auto-selection of first incident
- Performance optimization by conditionally rendering tab content
- Good error and loading state handling
- Consistent use of insights terminology and components
The TODO i18n comments (lines 83, 98, 130, 136) are noted and appear to be part of an incremental internationalization effort.
client/src/app/pages/insights/affected-applications/affected-applications.tsx (1)
151-152
: Fix the conditional rendering logicThe condition appears to be inverted. It should show the placeholder when fetching and no data is available yet.
Apply this diff to fix the logic:
- when={isFetching && !(currentPageAppReports || fetchError)} + when={!isFetching && !(currentPageAppReports || fetchError)}Likely an incorrect or invalid review comment.
client/src/app/Routes.tsx (1)
38-44
: LGTM! Clean separation of Issues and Insights componentsThe renaming of
AffectedApplications
toIssuesAffectedApplications
and the addition of new Insights components with their corresponding routes follows a consistent pattern and provides clear separation between the Issues and Insights features.Also applies to: 166-166, 189-213
client/src/app/pages/issues/helpers.ts (3)
3-5
: Good type system migration.The import updates properly migrate from the old
AnalysisIssue*
types to the newAnalysisInsight
andUiAnalysisReport*
types, maintaining type safety while aligning with the insights refactoring.
44-44
: Good cleanup of unused destructured variables.Removing the unused
tagCategories
andtags
from the destructuring assignment improves code cleanliness.
151-151
: Type parameter updates look correct.The function parameter type updates from
AnalysisRuleReport | AnalysisIssue | AnalysisIssueReport
to the new UI-specific types (UiAnalysisReportInsight | AnalysisInsight | UiAnalysisReportApplicationInsight
) maintain type safety while supporting the insights refactoring.Also applies to: 246-246, 264-268
client/src/app/pages/insights/insights-table.tsx (4)
78-80
: Simple and clear interface design.The props interface is well-defined with a clear mode discriminator that controls the table behavior.
87-97
: Good pattern for route-based state management.The application selection logic properly uses React Router to manage the selected application state via URL parameters, ensuring bookmarkable URLs and proper back/forward navigation.
268-270
: Good loading state handling.The loading condition properly handles the case where data is being fetched and no cached data or error exists.
284-306
: Excellent UX with conditional tooltip.The conditional tooltip for disabled application select provides clear feedback when no applications are available.
client/src/app/pages/insights/insights-page.tsx (4)
22-29
: Well-defined types and enums.The
InsightFilterGroups
enum andInsightsTabPath
type provide good type safety and clear intention for the component's API.
38-45
: Good route-based tab determination.The logic for determining the active tab from route matching is clear and handles the different possible route states properly.
63-71
: Excellent UX for filter preservation.The confirmation dialog when switching tabs with active filters provides good user experience by warning about filter reset. This prevents accidental loss of filter state.
90-112
: Clear and informative confirmation dialog.The confirmation dialog provides contextual messaging based on the target tab, making it clear what action will be taken.
client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/incident-code-snip-viewer.tsx (3)
47-55
: Line number parsing logic is robust.The parsing logic properly handles the regex matching and extracts line numbers and code content safely with proper validation.
58-61
: Good helper functions for line number conversion.The
absoluteToRelativeLineNum
andrelativeToAbsoluteLineNum
functions provide clear abstractions for converting between line number systems.
83-120
: Excellent error handling in Monaco integration.The try-catch block properly handles potential Monaco editor API failures and provides appropriate logging. The editor layout refresh is also correctly placed.
client/src/app/pages/insights/helpers.ts (2)
143-143
: Good naming consistency for insights.The constant name
FROM_INSIGHTS_PARAMS_KEY
properly follows the insights naming convention rather than issues.
265-273
: Naming consistency verifiedNo occurrences of
getIssueTitle
remain in the codebase, andgetInsightTitle
is correctly named in line with issue #2445. All related calls use the updated naming.client/src/app/api/rest/analysis.ts (4)
6-11
: Import updates align with new data models.The updated imports correctly reflect the new analysis data models introduced in
models.ts
, supporting the insights functionality.
36-39
: Type update correctly reflects the new AnalysisInsight model.The function now returns
AnalysisInsight
instead ofAnalysisIssue
, which aligns with the insights refactoring. The function namegetInsight
is already correct per the previous refactoring tracked in issue #2445.
50-54
: Return type correctly updated to AnalysisReportFile.The function now returns paginated results of
AnalysisReportFile
instead ofAnalysisFileReport
, consistent with the model renaming.
57-92
: All remaining function return types correctly updated.The type updates for all report functions are consistent with the new data models:
- Functions use
AnalysisReportInsight
for rule-level summaries- Functions use
AnalysisReportApplicationInsight
for application-specific insights- Functions use
AnalysisReportInsightApplication
for application/insight pairsThe separation between issues (effort>0) and insights (effort=0) is properly maintained through the URL filters.
client/src/app/api/models.ts (4)
11-17
: Well-designed utility type for UI table handling.The
WithUiId<T>
type provides a clean solution for adding client-side unique identifiers to objects that lack natural unique keys. The documentation clearly explains its purpose.
614-628
: Comprehensive AnalysisInsight interface properly structured.The interface includes all necessary fields for insights with appropriate optional modifiers. The structure supports the full insights functionality including incidents, links, facts, and labels.
630-638
: AnalysisIncident interface provides detailed incident tracking.The interface properly captures all necessary information for tracking specific occurrences of insights in source files, including location details and code context.
648-713
: All report interfaces are well-designed and properly documented.The report interfaces provide a comprehensive hierarchy:
AnalysisReportInsight
: Rule-level summaries with application countsAnalysisReportApplicationInsight
: Application-scoped insights with file countsAnalysisReportInsightApplication
: Application details for specific insightsAnalysisReportFile
: File-level incident summariesThe UI wrapper types (
UiAnalysisReportInsight
,UiAnalysisReportApplicationInsight
) correctly applyWithUiId
for table usage. Field-level documentation enhances clarity.client/src/app/queries/analysis.ts (3)
39-51
: Well-structured interfaces for standardizing query hook returns.The
AnalysisQueryResults<E>
andAnalysisQueryResult<E>
interfaces provide consistent structure for all analysis query hooks, properly distinguishing between paginated and single entity results.
83-107
: Improved type safety with dual generic parameters.The refactored
useFetchReportAll_
hook properly distinguishes between API data and UI-enhanced data using two generic parameters. The structured return usingAnalysisQueryResults<UI>
improves consistency.
221-239
: Hook updates properly implement new return interfaces.The
useFetchInsight
hook correctly returnsAnalysisQueryResult<AnalysisInsight>
, and the renameduseFetchIncidentsForInsight
better describes its purpose. All hooks consistently use the new standardized return types.Also applies to: 241-262
...ages/insights/insight-detail-drawer/file-incidents-detail-modal/file-all-incidents-table.tsx
Outdated
Show resolved
Hide resolved
...ges/insights/insight-detail-drawer/file-incidents-detail-modal/incident-code-snip-viewer.tsx
Outdated
Show resolved
Hide resolved
...ges/insights/insight-detail-drawer/file-incidents-detail-modal/incident-code-snip-viewer.tsx
Outdated
Show resolved
Hide resolved
client/src/app/pages/insights/insight-detail-drawer/insight-affected-files-table.tsx
Outdated
Show resolved
Hide resolved
client/src/app/pages/insights/insight-detail-drawer/insight-detail-drawer.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
🧹 Nitpick comments (5)
client/src/app/components/insights/components/insight-description-and-links.tsx (1)
27-31
: Consider using array index for list keys to handle potential duplicate URLs.Using
link.url
as the key could cause React warnings if the same URL appears multiple times in the links array.Apply this diff to use a more reliable key:
- {links.map((link) => ( - <ListItem key={link.url}> + {links.map((link, index) => ( + <ListItem key={`${link.url}-${index}`}>client/src/app/components/insights/components/insight-expanded-row-content.tsx (1)
47-67
: Consider consistent label rendering approach.The component uses
LabelGroup
for "otherLabels" but renders individualLabel
components for sources and targets. Consider usingLabelGroup
consistently for all label collections.Apply this diff for consistent label rendering:
<ExpandedFieldHeading>Target technologies</ExpandedFieldHeading> <div> - {targets.length > 0 - ? targets.map((target) => ( - <Label key={target} className={spacing.mrSm}> - {target} - </Label> - )) - : "None"} + {targets.length > 0 ? ( + <LabelGroup> + {targets.map((target) => ( + <Label key={target}> + {target} + </Label> + ))} + </LabelGroup> + ) : ( + "None" + )} </div> <ExpandedFieldHeading>Source technologies</ExpandedFieldHeading> <div> - {sources.length > 0 - ? sources.map((source) => ( - <Label key={source} className={spacing.mrSm}> - {source} - </Label> - )) - : "None"} + {sources.length > 0 ? ( + <LabelGroup> + {sources.map((source) => ( + <Label key={source}> + {source} + </Label> + ))} + </LabelGroup> + ) : ( + "None" + )} </div>Note: You can remove
className={spacing.mrSm}
from individual labels asLabelGroup
handles spacing between labels automatically.Also applies to: 75-88
client/src/app/components/insights/tables/use-insight-filters.ts (2)
27-35
: Add default case to switch statementThe switch statement should include a default case to handle unexpected filter group values gracefully.
const filters = groups.map((group) => { switch (group) { case InsightFilterGroups.ApplicationInventory: return applicationFilters; case InsightFilterGroups.Insights: return insightsFilters; + + default: + return []; } });
82-100
: Extract complex archetype filter logicThe archetype filter's
getServerFilterValue
function is complex and would benefit from being extracted into a separate helper function for better readability and testability.Consider extracting this logic into a helper function:
const getArchetypeApplicationIds = ( selectedOptions: string[] | undefined, archetypes: Archetype[] ): string[] => { if (!selectedOptions) return []; const findArchetypeByName = (name: string) => archetypes.find((item) => item.name === name); const getApplicationIds = (archetype: Archetype) => archetype.applications?.map((app) => String(app.id)) ?? []; return selectedOptions .map((option) => findArchetypeByName(option)) .filter(Boolean) .flatMap((archetype) => getApplicationIds(archetype)) .filter(Boolean); };Then use it in the filter configuration:
- getServerFilterValue: (selectedOptions) => { - const findArchetypeByName = (name: string) => { - return archetypes.find((item) => item.name === name); - }; - - const getApplicationIds = (archetype: Archetype) => { - return archetype.applications?.map((app) => String(app.id)); - }; - - if (!selectedOptions) return ["-1"]; - - const archetypeIds = selectedOptions - .map((option) => findArchetypeByName(option)) - .filter(Boolean) - .flatMap((archetype) => getApplicationIds(archetype)) - .filter(Boolean); - - return archetypeIds.length === 0 ? ["-1"] : archetypeIds; - }, + getServerFilterValue: (selectedOptions) => + getArchetypeApplicationIds(selectedOptions, archetypes),client/src/app/components/insights/helpers.ts (1)
110-126
: Add error handling for malformed label formats.The label parsing logic is correct but could be more robust. Consider handling cases where labels might not have the expected format after the prefix.
export const parseReportLabels = ( ruleReport: UiAnalysisReportInsight | UiAnalysisReportApplicationInsight ) => { const sources: string[] = []; const targets: string[] = []; const otherLabels: string[] = []; ruleReport.labels.forEach((label) => { if (label.startsWith("konveyor.io/source=")) { - sources.push(label.split("konveyor.io/source=")[1]); + const value = label.split("konveyor.io/source=")[1]; + if (value) sources.push(value); } else if (label.startsWith("konveyor.io/target=")) { - targets.push(label.split("konveyor.io/target=")[1]); + const value = label.split("konveyor.io/target=")[1]; + if (value) targets.push(value); } else { otherLabels.push(label); } }); return { sources, targets, otherLabels }; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
client/src/app/components/insights/components.ts
(1 hunks)client/src/app/components/insights/components/affected-apps-link.tsx
(1 hunks)client/src/app/components/insights/components/insight-description-and-links.tsx
(1 hunks)client/src/app/components/insights/components/insight-expanded-row-content.tsx
(1 hunks)client/src/app/components/insights/helpers.ts
(1 hunks)client/src/app/components/insights/tables/all-insights-table.tsx
(1 hunks)client/src/app/components/insights/tables/column-insight-title.tsx
(1 hunks)client/src/app/components/insights/tables/single-application-insights-table.tsx
(1 hunks)client/src/app/components/insights/tables/use-dynamic-columns.ts
(1 hunks)client/src/app/components/insights/tables/use-insight-filters.ts
(1 hunks)client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/file-incidents-detail-modal.tsx
(1 hunks)client/src/app/pages/insights/insights-page.tsx
(1 hunks)client/src/app/pages/issues/helpers.ts
(6 hunks)client/src/app/pages/issues/index.ts
(1 hunks)client/src/app/pages/issues/issues-page.tsx
(3 hunks)client/src/app/pages/issues/issues-table.tsx
(0 hunks)
💤 Files with no reviewable changes (1)
- client/src/app/pages/issues/issues-table.tsx
✅ Files skipped from review due to trivial changes (3)
- client/src/app/components/insights/components.ts
- client/src/app/pages/issues/index.ts
- client/src/app/components/insights/components/affected-apps-link.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- client/src/app/pages/insights/insight-detail-drawer/file-incidents-detail-modal/file-incidents-detail-modal.tsx
- client/src/app/pages/issues/helpers.ts
- client/src/app/pages/insights/insights-page.tsx
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: sjd78
PR: konveyor/tackle2-ui#2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like `getIssue` to `getInsight` and parameters like `issueId` to `insightId` in the analysis API client functions.
client/src/app/components/insights/tables/column-insight-title.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/issues-page.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/components/insights/helpers.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
🧬 Code Graph Analysis (5)
client/src/app/components/insights/tables/column-insight-title.tsx (1)
client/src/app/api/models.ts (1)
AnalysisInsight
(614-628)
client/src/app/components/insights/components/insight-description-and-links.tsx (2)
client/src/app/api/models.ts (1)
AnalysisInsightLink
(640-643)client/src/app/components/ExternalLink.tsx (1)
ExternalLink
(8-29)
client/src/app/components/insights/components/insight-expanded-row-content.tsx (3)
client/src/app/api/models.ts (2)
UiAnalysisReportInsight
(662-662)UiAnalysisReportApplicationInsight
(680-681)client/src/app/components/insights/helpers.ts (1)
parseReportLabels
(110-126)client/src/app/components/insights/components/insight-description-and-links.tsx (1)
InsightDescriptionAndLinks
(16-35)
client/src/app/components/insights/tables/all-insights-table.tsx (15)
client/src/app/components/insights/tables/use-dynamic-columns.ts (2)
TableColumns
(1-8)useDynamicColumns
(10-47)client/src/app/api/models.ts (2)
HubRequestParams
(31-41)UiAnalysisReportInsight
(662-662)client/src/app/queries/analysis.ts (1)
AnalysisQueryResults
(39-44)client/src/app/components/insights/tables/use-insight-filters.ts (1)
useInsightsTableFilters
(16-37)client/src/app/hooks/table-controls/useTableControlState.ts (1)
useTableControlState
(41-103)client/src/app/hooks/table-controls/getHubRequestParams.ts (1)
getHubRequestParams
(32-44)client/src/app/hooks/table-controls/useTableControlProps.ts (1)
useTableControlProps
(23-177)client/src/app/Constants.ts (1)
UI_UNIQUE_ID
(27-27)client/src/app/components/FilterToolbar/FilterToolbar.tsx (1)
FilterToolbar
(117-243)client/src/app/components/TableControls/TableHeaderContentWithControls.tsx (1)
TableHeaderContentWithControls
(10-26)client/src/app/components/insights/helpers.ts (1)
parseReportLabels
(110-126)client/src/app/components/TableControls/TableRowContentWithControls.tsx (1)
TableRowContentWithControls
(28-54)client/src/app/components/insights/tables/column-insight-title.tsx (1)
InsightTitleColumn
(4-11)client/src/app/components/insights/components/affected-apps-link.tsx (1)
AffectedAppsLink
(15-33)client/src/app/components/insights/components/insight-expanded-row-content.tsx (1)
InsightExpandedRowContent
(33-100)
client/src/app/components/insights/helpers.ts (5)
client/src/app/components/FilterToolbar/FilterToolbar.tsx (1)
FilterValue
(25-25)client/src/app/api/models.ts (3)
UiAnalysisReportInsight
(662-662)UiAnalysisReportApplicationInsight
(680-681)AnalysisInsight
(614-628)client/src/app/Paths.ts (1)
Paths
(82-82)client/src/app/hooks/useUrlParams.ts (1)
trimAndStringifyUrlParams
(125-151)client/src/app/hooks/table-controls/filtering/helpers.ts (2)
serializeFilterUrlParams
(8-27)deserializeFilterUrlParams
(33-43)
⏰ 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 (9)
client/src/app/pages/issues/issues-page.tsx (2)
20-26
: Good refactoring to use specialized tables, but naming inconsistency persists.The replacement of the generic
IssuesTable
with specializedAllInsightsTable
andSingleApplicationInsightsTable
components improves modularity. However, there's a naming inconsistency where "insights" tables are used in an "issues" page context.Per the retrieved learning, this naming inconsistency is being tracked in issue #2445.
Also applies to: 95-117
52-52
: Correct fix for the useEffect dependency array.Adding
history
to the dependency array prevents potential stale closure issues.client/src/app/components/insights/tables/use-dynamic-columns.ts (1)
10-47
: Well-implemented dynamic column configuration hook.The
useDynamicColumns
hook provides a clean and flexible way to configure table columns with:
- Proper default value handling
- Support for boolean visibility flags and custom string labels
- Type-safe implementation
The implementation follows React hooks best practices and TypeScript patterns effectively.
client/src/app/components/insights/tables/all-insights-table.tsx (1)
122-124
: Improve loading state logicThe current loading check might show the placeholder when there's an error. Consider checking for loading state without an error.
- if (isLoading && !(currentPageItems || fetchError)) { + if (isLoading && !fetchError) { return <AppPlaceholder />; }Likely an incorrect or invalid review comment.
client/src/app/components/insights/tables/single-application-insights-table.tsx (1)
187-189
: Improve loading state logicSimilar to the AllInsightsTable, the loading check might show the placeholder when there's an error.
- if (isLoading && !(currentPageItems || fetchError)) { + if (isLoading && !fetchError) { return <AppPlaceholder />; }Likely an incorrect or invalid review comment.
client/src/app/components/insights/helpers.ts (4)
16-28
: Well-structured constants and type definitions.The filter keys, type alias, and parameter key are clearly defined with good documentation. The
~
prefix for URL parameter sorting is a nice touch for readability.
61-95
: Well-implemented URL state restoration logic.The function correctly handles the complex task of restoring original URL parameters while preserving filter changes. The use of
null
for clearing filters and the proper deserialization/serialization flow demonstrates good understanding of the URL parameter management.
97-108
: Simple and correct URL generation for dependencies.The function properly constructs a filtered dependencies URL using the established utility functions and naming conventions.
128-136
: Robust title extraction with appropriate fallbacks.The function correctly implements a fallback chain for extracting insight titles, handling multiple input types and edge cases gracefully.
client/src/app/components/insights/tables/all-insights-table.tsx
Outdated
Show resolved
Hide resolved
client/src/app/components/insights/tables/all-insights-table.tsx
Outdated
Show resolved
Hide resolved
client/src/app/components/insights/tables/column-insight-title.tsx
Outdated
Show resolved
Hide resolved
client/src/app/components/insights/tables/single-application-insights-table.tsx
Outdated
Show resolved
Hide resolved
client/src/app/components/insights/tables/single-application-insights-table.tsx
Show resolved
Hide resolved
client/src/app/components/insights/tables/single-application-insights-table.tsx
Outdated
Show resolved
Hide resolved
client/src/app/components/insights/tables/use-insight-filters.ts
Outdated
Show resolved
Hide resolved
client/src/app/components/insights/tables/use-insight-filters.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
🔭 Outside diff range comments (1)
client/src/app/pages/issues/file-incidents-detail-modal/file-incidents-detail-modal.tsx (1)
62-62
: Update function name to align with insights terminology.The function
getIssueTitle
should be renamed togetInsightTitle
to maintain consistency with the insights refactoring.- const issueTitle = getIssueTitle(issue); + const insightTitle = getInsightTitle(issue);Also update the variable usage throughout the component:
- issueTitle={issueTitle} + insightTitle={insightTitle}- <Truncate content={issueTitle} /> + <Truncate content={insightTitle} />
🧹 Nitpick comments (8)
client/src/app/pages/insights/file-incidents-detail-modal/file-all-incidents-table.tsx (2)
140-144
: Improve robustness of message processing.The
getFirstNonEmptyLine
function could be more robust by handling edge cases like null/undefined messages.-const getFirstNonEmptyLine = (message: string): string | null => { +const getFirstNonEmptyLine = (message: string | null | undefined): string | null => { + if (!message) return null; const lines = message.split("\n"); const nonEmptyLine = lines.find((line) => line.trim() !== ""); return nonEmptyLine || null; };
146-154
: Enhance message display component error handling.The message display component should handle potential ReactMarkdown rendering errors gracefully.
const messageDisplayComponent = (message: string) => { const content = getFirstNonEmptyLine(message); if (content === null) { return <div className="empty-cell">No content available.</div>; } - return ( - <ReactMarkdown components={markdownPFComponents}>{content}</ReactMarkdown> - ); + try { + return ( + <ReactMarkdown components={markdownPFComponents}>{content}</ReactMarkdown> + ); + } catch (error) { + console.warn("Failed to render markdown content:", error); + return <div className="markdown-error">{content}</div>; + } };client/src/app/pages/insights/insight-affected-files-table.tsx (1)
58-60
: Consider extracting hardcoded text for internationalization.The filter placeholder text contains hardcoded English text that should be internationalized.
placeholderText: - t("actions.filterBy", { - what: "file", // TODO i18n - }) + "...", + t("actions.filterBy", { + what: t("terms.file") + }) + "...",Make sure to add the corresponding translation key in the localization files.
client/src/app/pages/insights/affected-applications-page.tsx (1)
136-140
: Extract location hook usage to avoid multiple calls.The
useLocation()
hook is called inside the component parameter, which could lead to unnecessary re-renders.+ const location = useLocation(); + return ( <> <PageSection variant={PageSectionVariants.light}> <TextContent> <Text component="h1">{t("terms.affectedApplications")}</Text> </TextContent> <Breadcrumb> <BreadcrumbItem> <Link to={getBackToAllInsightsUrl({ fromFilterValues: filterValues, - fromLocation: useLocation(), + fromLocation: location, })} >client/src/app/pages/insights/file-incidents-detail-modal/incident-code-snip-viewer.tsx (2)
17-17
: Improve regex pattern robustness.The regex pattern for parsing code lines could be more robust to handle edge cases and provide better error handling.
-const codeLineRegex = /^\s*([0-9]+)( {2})?(.*)$/; // Pattern: leading whitespace (line number) (2 spaces)? (code) +const codeLineRegex = /^\s*(\d+)(\s{2})?(.*)$/; // Pattern: leading whitespace (line number) (optional 2+ spaces) (code)The
\d+
is more explicit than[0-9]+
and\s{2}
is more flexible than( {2})?
.
94-98
: Use Monaco's built-in column detection methods.The hard-coded column calculations could be replaced with Monaco's built-in methods for better accuracy.
startLineNumber: relativeLineNum, - startColumn: - model?.getLineFirstNonWhitespaceColumn(relativeLineNum), + startColumn: model.getLineFirstNonWhitespaceColumn(relativeLineNum) || 1, endLineNumber: relativeLineNum, - endColumn: - model?.getLineLastNonWhitespaceColumn(relativeLineNum), + endColumn: model.getLineLastNonWhitespaceColumn(relativeLineNum) || model.getLineMaxColumn(relativeLineNum),client/src/app/pages/insights/file-incidents-detail-modal/file-incidents-detail-modal.tsx (1)
93-144
: Consider simplifying the tabs array structure.The nested array with
.flat(1).filter(Boolean)
is complex and could be made more readable.- <Tabs - activeKey={activeTabIncidentId} - onSelect={(_event, tabKey) => - setActiveTabIncidentId(tabKey as IncidentIdOrAll) - } - > - {[ - firstFiveIncidents.map((incident, index) => ( - <Tab - key={incident.id} - eventKey={incident.id} - title={`Incident #${index + 1}: Line ${incident.line}`} - > - {/* content */} - </Tab> - )), - totalNumIncidents > 5 && [ - <Tab - key="all" - eventKey="all" - title={`All incidents (${totalNumIncidents})`} - > - {/* content */} - </Tab>, - ], - ] - .flat(1) - .filter(Boolean)} - </Tabs> + <Tabs + activeKey={activeTabIncidentId} + onSelect={(_event, tabKey) => + setActiveTabIncidentId(tabKey as IncidentIdOrAll) + } + > + {firstFiveIncidents.map((incident, index) => ( + <Tab + key={incident.id} + eventKey={incident.id} + title={`Incident #${index + 1}: Line ${incident.line}`} + > + {/* content */} + </Tab> + ))} + {totalNumIncidents > 5 && ( + <Tab + key="all" + eventKey="all" + title={`All incidents (${totalNumIncidents})`} + > + {/* content */} + </Tab> + )} + </Tabs>client/src/app/pages/insights/helpers.ts (1)
145-180
: Complex but well-documented URL parameter management.The function correctly handles the sophisticated requirement of preserving and overriding filter state between pages. The logic is complex but necessary for the user experience.
Consider extracting some of the parameter manipulation logic into smaller helper functions for better maintainability if this pattern is used elsewhere.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
client/src/app/Routes.tsx
(3 hunks)client/src/app/components/BreakingPathDisplay.tsx
(2 hunks)client/src/app/components/insights/components/affected-apps-link.tsx
(1 hunks)client/src/app/components/insights/helpers.ts
(1 hunks)client/src/app/components/insights/tables/all-insights-table.tsx
(1 hunks)client/src/app/components/insights/tables/single-application-insights-table.tsx
(1 hunks)client/src/app/pages/insights/affected-applications-page.tsx
(1 hunks)client/src/app/pages/insights/file-incidents-detail-modal/file-all-incidents-table.tsx
(1 hunks)client/src/app/pages/insights/file-incidents-detail-modal/file-incidents-detail-modal.tsx
(1 hunks)client/src/app/pages/insights/file-incidents-detail-modal/incident-code-snip-viewer.tsx
(1 hunks)client/src/app/pages/insights/helpers.ts
(1 hunks)client/src/app/pages/insights/insight-affected-files-table.tsx
(1 hunks)client/src/app/pages/insights/insight-detail-drawer.tsx
(1 hunks)client/src/app/pages/insights/insights-page.tsx
(1 hunks)client/src/app/pages/issues/affected-applications-page.tsx
(2 hunks)client/src/app/pages/issues/affected-applications/index.ts
(0 hunks)client/src/app/pages/issues/file-incidents-detail-modal/file-all-incidents-table.css
(1 hunks)client/src/app/pages/issues/file-incidents-detail-modal/file-all-incidents-table.tsx
(3 hunks)client/src/app/pages/issues/file-incidents-detail-modal/file-incidents-detail-modal.tsx
(3 hunks)client/src/app/pages/issues/file-incidents-detail-modal/incident-code-snip-viewer.css
(1 hunks)client/src/app/pages/issues/file-incidents-detail-modal/index.ts
(1 hunks)client/src/app/pages/issues/helpers.ts
(4 hunks)client/src/app/pages/issues/index.ts
(0 hunks)client/src/app/pages/issues/issue-affected-files-table.tsx
(6 hunks)client/src/app/pages/issues/issue-detail-drawer.tsx
(1 hunks)client/src/app/pages/issues/issue-detail-drawer/index.ts
(0 hunks)client/src/app/pages/issues/issues-page.tsx
(4 hunks)
💤 Files with no reviewable changes (3)
- client/src/app/pages/issues/affected-applications/index.ts
- client/src/app/pages/issues/issue-detail-drawer/index.ts
- client/src/app/pages/issues/index.ts
✅ Files skipped from review due to trivial changes (5)
- client/src/app/pages/issues/issue-detail-drawer.tsx
- client/src/app/pages/issues/file-incidents-detail-modal/index.ts
- client/src/app/pages/issues/file-incidents-detail-modal/incident-code-snip-viewer.css
- client/src/app/pages/issues/file-incidents-detail-modal/file-all-incidents-table.css
- client/src/app/components/BreakingPathDisplay.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- client/src/app/components/insights/components/affected-apps-link.tsx
- client/src/app/Routes.tsx
- client/src/app/components/insights/tables/single-application-insights-table.tsx
- client/src/app/components/insights/helpers.ts
- client/src/app/components/insights/tables/all-insights-table.tsx
- client/src/app/pages/issues/helpers.ts
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: sjd78
PR: konveyor/tackle2-ui#2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like `getIssue` to `getInsight` and parameters like `issueId` to `insightId` in the analysis API client functions.
client/src/app/pages/issues/file-incidents-detail-modal/file-all-incidents-table.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/issues-page.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/insights/helpers.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/issue-affected-files-table.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/issues/file-incidents-detail-modal/file-incidents-detail-modal.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
🧬 Code Graph Analysis (2)
client/src/app/pages/issues/file-incidents-detail-modal/file-all-incidents-table.tsx (2)
client/src/app/api/models.ts (1)
AnalysisReportFile
(707-713)client/src/app/queries/analysis.ts (1)
useFetchIncidentsForInsight
(241-262)
client/src/app/pages/issues/issue-affected-files-table.tsx (3)
client/src/app/api/models.ts (3)
AnalysisInsight
(614-628)WithUiId
(17-17)AnalysisReportFile
(707-713)client/src/app/Constants.ts (1)
UI_UNIQUE_ID
(27-27)client/src/app/components/BreakingPathDisplay.tsx (1)
BreakingPathDisplay
(13-29)
⏰ 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 (23)
client/src/app/pages/issues/affected-applications-page.tsx (1)
35-36
: LGTM! Clean refactoring with consistent naming.The component renaming to
AffectedApplicationsPage
, updated relative import paths, and addition of default export align well with the broader restructuring effort. The changes maintain functionality while improving code organization.Also applies to: 44-44, 248-248
client/src/app/pages/issues/file-incidents-detail-modal/file-all-incidents-table.tsx (2)
5-6
: LGTM! Type updates align with insights refactoring.The import changes from
AnalysisFileReport
toAnalysisReportFile
anduseFetchIncidents
touseFetchIncidentsForInsight
properly reflect the updated data model and API hook naming conventions in the insights refactoring effort.Also applies to: 22-22
47-48
: Correct hook usage with updated data model.The switch to
useFetchIncidentsForInsight
withfileReport.insightId
parameter correctly utilizes the updatedAnalysisReportFile
interface and aligns with the insight-focused data model.client/src/app/pages/issues/issues-page.tsx (4)
20-27
: Import updates align with table component refactoring.The replacement of the monolithic
IssuesTable
with specializedAllInsightsTable
andSingleApplicationInsightsTable
components, along with updated query hooks, improves modularity and aligns with the insights-focused architecture.
52-52
: Correct dependency array update.Adding
history
to the dependency array is the right fix since it's used within the effect body. This follows React best practices for exhaustive deps.
57-58
: Clean state management for detail drawer.The new
issueInDetailDrawer
state variable with proper typing provides a clean way to manage which issue is currently displayed in the detail drawer.
95-124
: Excellent refactoring to specialized table components.The replacement of the monolithic
IssuesTable
withAllInsightsTable
andSingleApplicationInsightsTable
improves modularity and maintainability. Each table receives appropriate props for its specific use case, and theIssueDetailDrawer
integration with proper state management is well-implemented.client/src/app/pages/issues/issue-affected-files-table.tsx (4)
11-11
: Type and import updates align with insights refactoring.The updates from
AnalysisIssue
toAnalysisInsight
,AnalysisFileReport
toAnalysisReportFile
, and the addition ofWithUiId
andUI_UNIQUE_ID
properly align with the broader insights refactoring. TheBreakingPathDisplay
import suggests improved file path rendering.Also applies to: 18-18, 27-27, 30-30
85-85
: Improved unique identification with UI_UNIQUE_ID.Using
UI_UNIQUE_ID
as theidProperty
instead of file paths provides better uniqueness guarantees for table row identification and aligns with theWithUiId
type pattern.
107-107
: Consistent state typing with WithUiId pattern.The state type update to
WithUiId<AnalysisReportFile>
maintains consistency with the new data model and ensures proper unique identification for the modal component.
152-155
: UI improvements with consistent unique identification.The table row key update to use
fileReport[UI_UNIQUE_ID]
maintains consistency with the unique identifier pattern, and theBreakingPathDisplay
component usage improves file path readability with proper line breaking.Also applies to: 167-169
client/src/app/pages/insights/insight-detail-drawer.tsx (1)
1-85
: Well-structured new insight detail drawer component.This new
InsightDetailDrawer
component follows React best practices with:
- Proper TypeScript interfaces and typing
- Clean separation of concerns with dedicated table component
- Appropriate loading and no-data state handling
- Internationalization support
- Extensible tab structure for future enhancements
- Integration with the insights-focused data model
The implementation is clean, maintainable, and aligns well with the existing codebase patterns.
client/src/app/pages/insights/insight-affected-files-table.tsx (1)
1-207
: LGTM! Well-structured component implementation.This component demonstrates excellent use of established patterns:
- Proper separation of concerns with table controls
- Comprehensive state management with URL persistence
- Good error handling and loading states
- Clean integration with the modal system
- Consistent use of PatternFly components
The implementation follows the codebase conventions and integrates well with the insights refactoring.
client/src/app/pages/insights/affected-applications-page.tsx (1)
84-92
: Please confirminsight.ruleset
andinsight.rule
filter keys with the backend API
We didn’t find any matching schema or GraphQL definitions in the client repo. Make sure these implicit filter field names are exactly what the backend expects.
• client/src/app/pages/insights/affected-applications-page.tsx (lines 84–92)
• client/src/app/pages/issues/affected-applications-page.tsx (lines 84–92)client/src/app/pages/insights/file-incidents-detail-modal/incident-code-snip-viewer.tsx (1)
19-21
: Maintain consistent naming with insights terminology.The interface uses
insightTitle
which is consistent with the insights refactoring, but ensure this aligns with usage throughout the component.The naming convention here is correct and consistent with the insights refactoring effort.
client/src/app/pages/insights/file-incidents-detail-modal/file-incidents-detail-modal.tsx (3)
1-26
: LGTM! Clean imports and proper component organization.The imports are well-organized and use the correct renamed hooks (
useFetchIncidentsForInsight
) that align with the insights refactoring.
27-36
: Well-structured component interface.The props interface is properly typed and follows established naming conventions.
38-62
: Efficient data fetching strategy with proper state management.The approach of fetching only the first 5 incidents initially is a good performance optimization. The auto-selection logic and loading state handling are implemented correctly.
client/src/app/pages/insights/insights-page.tsx (3)
1-37
: Well-organized imports and type definitions.The imports are clean and the
InsightFilterGroups
enum andInsightsTabPath
type provide good type safety for the component.
69-91
: Excellent user experience with filter confirmation.The logic properly checks for existing filters and shows a confirmation dialog to prevent accidental filter loss when switching tabs.
100-102
: effort column visibility confirmedThe
columns
prop is consumed byuseDynamicColumns
, which merges your overrides into a default set and only renders columns with a value oftrue
(or a custom string for header text). By settingeffort: false
, you’re explicitly hiding the Effort column—this prop does not affect data filtering. No changes are required.client/src/app/pages/insights/helpers.ts (2)
1-37
: Well-structured imports and type definitions.The imports are comprehensive and the
filterKeysToCarry
constant with theInsightsFilterValuesToCarry
type provide good type safety for filter management between pages.
182-190
: Clean helper function with good fallback logic.The function properly handles missing or undefined values with sensible fallbacks.
client/src/app/pages/insights/file-incidents-detail-modal/file-all-incidents-table.tsx
Outdated
Show resolved
Hide resolved
client/src/app/pages/insights/file-incidents-detail-modal/file-incidents-detail-modal.tsx
Outdated
Show resolved
Hide resolved
client/src/app/pages/insights/file-incidents-detail-modal/file-incidents-detail-modal.tsx
Show resolved
Hide resolved
client/src/app/pages/insights/file-incidents-detail-modal/file-incidents-detail-modal.tsx
Show resolved
Hide resolved
client/src/app/pages/insights/file-incidents-detail-modal/incident-code-snip-viewer.tsx
Show resolved
Hide resolved
client/src/app/pages/issues/file-incidents-detail-modal/file-incidents-detail-modal.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (4)
client/src/app/components/insights/tables/use-insight-filters.ts (2)
59-59
: Inconsistent filterGroup parameter usage (duplicate issue).The filterGroup property is hardcoded to
InsightFilterGroups.ApplicationInventory
instead of using the passedfilterGroupLabel
parameter, making the parameter ineffective.Also applies to: 73-73, 106-106, 118-118
91-99
: Magic number "-1" used for invalid application IDs (duplicate issue).Using "-1" as a magic value for "no matching applications" relies on the assumption that "-1" is an invalid application ID, which is not ideal.
client/src/app/pages/insights/insights-page.tsx (2)
29-32
: Duplicate enum definition (already flagged in other file).This enum is already defined in
use-insight-filters.ts
. See the comment in that file for the recommended solution.
123-145
: Dialog strings remain hardcoded (acknowledged by user).Based on past feedback, internationalization of these strings was deferred. This is noted for future reference.
🧹 Nitpick comments (4)
client/src/app/components/insights/tables/use-insight-filters.ts (1)
174-182
: Potential case-sensitivity issue in source filter.The source filter logic checks for "None" and "none" separately, but this approach is fragile and could miss other case variations.
Consider using a case-insensitive comparison:
- if (value?.[0] === "None" || value?.[0] === "none") { + if (value?.[0]?.toLowerCase() === "none") {client/src/app/pages/insights/insights-page.tsx (3)
61-63
: Hardcoded description text should be internationalized.While past feedback indicated internationalization wasn't a priority "not now", this hardcoded description text creates inconsistency since the title is already internationalized.
For consistency with the internationalized title, consider:
- <Text component="small"> - This report provides a concise summary of all insights identified. - </Text> + <Text component="small"> + {t("message.insightsDescription")} + </Text>
80-86
: Tab titles should be internationalized for consistency.The tab titles use hardcoded English strings while other UI elements are internationalized.
Consider internationalizing the tab titles:
<Tab eventKey={Paths.insightsAllTab} - title={<TabTitleText>All insights</TabTitleText>} + title={<TabTitleText>{t("terms.allInsights")}</TabTitleText>} /> <Tab eventKey={Paths.insightsSingleAppTab} - title={<TabTitleText>Single application</TabTitleText>} + title={<TabTitleText>{t("terms.singleApplication")}</TabTitleText>} />
69-71
: Filter detection logic could be more robust.The current logic only checks for the existence of the filters parameter but doesn't verify if filters are actually active (could be empty).
Consider checking if filters are actually applied:
- const pageHasFilters = new URLSearchParams(location.search).has( - `${TablePersistenceKeyPrefix.insights}:filters` - ); + const searchParams = new URLSearchParams(location.search); + const filtersParam = searchParams.get(`${TablePersistenceKeyPrefix.insights}:filters`); + const pageHasFilters = filtersParam && JSON.parse(filtersParam).length > 0;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
client/src/app/components/insights/tables/all-insights-table.tsx
(1 hunks)client/src/app/components/insights/tables/column-insight-title.tsx
(1 hunks)client/src/app/components/insights/tables/single-application-insights-table.tsx
(1 hunks)client/src/app/components/insights/tables/use-insight-filters.ts
(1 hunks)client/src/app/layout/SidebarApp/SidebarApp.tsx
(1 hunks)client/src/app/pages/insights/file-incidents-detail-modal/file-all-incidents-table.tsx
(1 hunks)client/src/app/pages/insights/file-incidents-detail-modal/file-incidents-detail-modal.tsx
(1 hunks)client/src/app/pages/insights/insights-page.tsx
(1 hunks)client/src/app/pages/issues/issues-page.tsx
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- client/src/app/layout/SidebarApp/SidebarApp.tsx
- client/src/app/components/insights/tables/column-insight-title.tsx
- client/src/app/pages/insights/file-incidents-detail-modal/file-all-incidents-table.tsx
- client/src/app/pages/issues/issues-page.tsx
- client/src/app/components/insights/tables/all-insights-table.tsx
- client/src/app/components/insights/tables/single-application-insights-table.tsx
- client/src/app/pages/insights/file-incidents-detail-modal/file-incidents-detail-modal.tsx
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: sjd78
PR: konveyor/tackle2-ui#2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like `getIssue` to `getInsight` and parameters like `issueId` to `insightId` in the analysis API client functions.
client/src/app/components/insights/tables/use-insight-filters.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/pages/insights/insights-page.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
🧬 Code Graph Analysis (1)
client/src/app/components/insights/tables/use-insight-filters.ts (7)
client/src/app/components/FilterToolbar/FilterToolbar.tsx (1)
FilterCategory
(83-87)client/src/app/utils/utils.ts (1)
universalComparator
(218-224)client/src/app/queries/applications.ts (1)
useFetchApplications
(37-61)client/src/app/queries/archetypes.ts (1)
useFetchArchetypes
(20-48)client/src/app/queries/businessservices.ts (1)
useFetchBusinessServices
(17-32)client/src/app/queries/tags.ts (1)
useFetchTagsWithTagItems
(65-110)client/src/app/api/models.ts (1)
Archetype
(871-885)
⏰ 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: build-and-upload-for-global-ci
- GitHub Check: unit-test
client/src/app/components/insights/tables/use-insight-filters.ts
Outdated
Show resolved
Hide resolved
client/src/app/components/insights/tables/use-insight-filters.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (6)
client/src/app/components/insights/tables/use-insight-filters.ts (5)
11-14
: Duplicate enum definition detected.The
InsightFilterGroups
enum is defined in both this file and ininsights-page.tsx
, creating code duplication and potential maintenance issues.
70-73
: Use consistent filterGroup parameter.The filterGroup should use the passed
filterGroupLabel
parameter instead of the enum value directly for consistency.
91-99
: Avoid magic number "-1" for invalid application IDs.Using "-1" as a magic value for "no matching applications" is not ideal. Consider using an empty array or a more descriptive approach.
103-106
: Use consistent filterGroup parameter.Similar to the previous comment, this filterGroup assignment should use the passed parameter for consistency.
115-118
: Use consistent filterGroup parameter.The filterGroup should use the passed
filterGroupLabel
parameter for consistency.client/src/app/pages/insights/helpers.ts (1)
72-105
: Review archetype filter logic and magic number usage.The archetype filter has the same issues previously identified: misleading categoryKey "application.id" for archetypes, and magic number "-1" usage as fallback values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
client/src/app/components/insights/tables/use-insight-filters.ts
(1 hunks)client/src/app/pages/insights/helpers.ts
(1 hunks)client/src/app/pages/insights/insights-page.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- client/src/app/pages/insights/insights-page.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: sjd78
PR: konveyor/tackle2-ui#2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like `getIssue` to `getInsight` and parameters like `issueId` to `insightId` in the analysis API client functions.
client/src/app/pages/insights/helpers.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
⏰ 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: build-and-upload-for-global-ci
- GitHub Check: unit-test
🔇 Additional comments (8)
client/src/app/components/insights/tables/use-insight-filters.ts (3)
16-37
: LGTM! Clean hook implementation.The main hook correctly maps filter groups to their respective filter categories and properly flattens the results.
39-43
: Well-implemented helper function.Clean transformation logic with proper sorting for select options.
143-203
: LGTM! Insights filters implementation looks correct.The server filter value transformations properly handle array access and wildcard usage consistently across all filter types.
client/src/app/pages/insights/helpers.ts (5)
36-41
: Well-defined filter carrying mechanism.Good approach to explicitly define which filter keys should be carried between pages with proper typing.
52-71
: Clean application name filter implementation.The application name filter is well-implemented with proper sorting and server filter value transformation.
106-144
: Business service and tags filters look good.Both filters have clean implementations with appropriate server filter value transformations.
147-184
: Sophisticated URL state management implementation.The
getBackToAllInsightsUrl
function correctly handles the complex requirement of preserving filter state between insights and affected applications pages, with proper parameter serialization/deserialization.
186-194
: Robust title extraction utility.The
getInsightTitle
function has good fallback logic for extracting titles from insight reports with appropriate handling of missing 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]>
Signed-off-by: Scott J Dickerson <[email protected]>
Signed-off-by: Scott J Dickerson <[email protected]>
…mers Signed-off-by: Scott J Dickerson <[email protected]>
…ues and insights 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]>
… issues pages 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]>
Signed-off-by: Scott J Dickerson <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
client/src/app/components/insights/tables/single-application-insights-table.tsx (2)
230-230
: Fix inconsistent null check for selectedAppIdThe
selectedAppId
is typed asnumber | undefined
from the hook, but you're checking againstnull
. This should beundefined
for consistency with the type definition.- isDisabled={selectedAppId === null} + isDisabled={selectedAppId === undefined}
273-273
: Fix inconsistent null check for selectedAppIdSimilar to line 230, this should check for
undefined
instead ofnull
to match the type definition.- isNoData={totalItemCount === 0 || selectedAppId === null} + isNoData={totalItemCount === 0 || selectedAppId === undefined}
🧹 Nitpick comments (1)
client/src/app/components/insights/tables/single-application-insights-table.tsx (1)
261-261
: Update tooltip message key for insights contextThe tooltip is using
"message.issuesEffortTooltip"
which refers to "issues" but this is now an insights table. Consider updating to use an insights-specific tooltip key or verify the message content is appropriate for insights.- tooltip: `${t("message.issuesEffortTooltip")}`, + tooltip: `${t("message.insightsEffortTooltip")}`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
client/public/locales/en/translation.json
(2 hunks)client/src/app/Constants.ts
(1 hunks)client/src/app/Paths.ts
(1 hunks)client/src/app/Routes.tsx
(3 hunks)client/src/app/api/models.ts
(2 hunks)client/src/app/api/rest/analysis.ts
(4 hunks)client/src/app/components/BreakingPathDisplay.tsx
(2 hunks)client/src/app/components/insights/components.ts
(1 hunks)client/src/app/components/insights/components/affected-apps-link.tsx
(1 hunks)client/src/app/components/insights/components/insight-description-and-links.tsx
(1 hunks)client/src/app/components/insights/components/insight-expanded-row-content.tsx
(1 hunks)client/src/app/components/insights/helpers.ts
(1 hunks)client/src/app/components/insights/tables/all-insights-table.tsx
(1 hunks)client/src/app/components/insights/tables/column-insight-title.tsx
(1 hunks)client/src/app/components/insights/tables/single-application-insights-table.tsx
(1 hunks)client/src/app/components/insights/tables/use-dynamic-columns.ts
(1 hunks)client/src/app/components/insights/tables/use-insight-filters.ts
(1 hunks)client/src/app/layout/SidebarApp/SidebarApp.tsx
(1 hunks)client/src/app/pages/insights/affected-applications-page.tsx
(1 hunks)client/src/app/pages/insights/file-incidents-detail-modal/file-all-incidents-table.tsx
(1 hunks)client/src/app/pages/insights/file-incidents-detail-modal/file-incidents-detail-modal.tsx
(1 hunks)client/src/app/pages/insights/file-incidents-detail-modal/incident-code-snip-viewer.tsx
(1 hunks)client/src/app/pages/insights/helpers.ts
(1 hunks)client/src/app/pages/insights/insight-affected-files-table.tsx
(1 hunks)client/src/app/pages/insights/insight-detail-drawer.tsx
(1 hunks)client/src/app/pages/insights/insights-page.tsx
(1 hunks)client/src/app/pages/issues/affected-applications-page.tsx
(2 hunks)client/src/app/pages/issues/affected-applications/index.ts
(0 hunks)client/src/app/pages/issues/file-incidents-detail-modal/file-all-incidents-table.css
(1 hunks)client/src/app/pages/issues/file-incidents-detail-modal/file-all-incidents-table.tsx
(3 hunks)client/src/app/pages/issues/file-incidents-detail-modal/file-incidents-detail-modal.tsx
(3 hunks)client/src/app/pages/issues/file-incidents-detail-modal/incident-code-snip-viewer.css
(1 hunks)client/src/app/pages/issues/file-incidents-detail-modal/index.ts
(1 hunks)client/src/app/pages/issues/helpers.ts
(4 hunks)client/src/app/pages/issues/index.ts
(0 hunks)client/src/app/pages/issues/issue-affected-files-table.tsx
(6 hunks)client/src/app/pages/issues/issue-detail-drawer.tsx
(1 hunks)client/src/app/pages/issues/issue-detail-drawer/index.ts
(0 hunks)client/src/app/pages/issues/issues-page.tsx
(4 hunks)client/src/app/pages/issues/issues-table.tsx
(0 hunks)client/src/app/queries/analysis.ts
(13 hunks)client/src/app/utils/query-utils.ts
(1 hunks)
💤 Files with no reviewable changes (4)
- client/src/app/pages/issues/affected-applications/index.ts
- client/src/app/pages/issues/issue-detail-drawer/index.ts
- client/src/app/pages/issues/issues-table.tsx
- client/src/app/pages/issues/index.ts
✅ Files skipped from review due to trivial changes (1)
- client/src/app/components/insights/tables/column-insight-title.tsx
🚧 Files skipped from review as they are similar to previous changes (35)
- client/src/app/pages/issues/file-incidents-detail-modal/file-all-incidents-table.css
- client/src/app/layout/SidebarApp/SidebarApp.tsx
- client/src/app/components/insights/components.ts
- client/src/app/components/insights/components/insight-description-and-links.tsx
- client/public/locales/en/translation.json
- client/src/app/pages/issues/issue-detail-drawer.tsx
- client/src/app/pages/issues/file-incidents-detail-modal/incident-code-snip-viewer.css
- client/src/app/pages/issues/issues-page.tsx
- client/src/app/pages/issues/affected-applications-page.tsx
- client/src/app/Constants.ts
- client/src/app/Paths.ts
- client/src/app/pages/issues/file-incidents-detail-modal/file-all-incidents-table.tsx
- client/src/app/components/BreakingPathDisplay.tsx
- client/src/app/pages/insights/file-incidents-detail-modal/file-incidents-detail-modal.tsx
- client/src/app/pages/issues/file-incidents-detail-modal/file-incidents-detail-modal.tsx
- client/src/app/components/insights/helpers.ts
- client/src/app/components/insights/tables/use-insight-filters.ts
- client/src/app/pages/insights/insights-page.tsx
- client/src/app/pages/insights/file-incidents-detail-modal/incident-code-snip-viewer.tsx
- client/src/app/components/insights/tables/all-insights-table.tsx
- client/src/app/pages/insights/helpers.ts
- client/src/app/Routes.tsx
- client/src/app/utils/query-utils.ts
- client/src/app/pages/issues/issue-affected-files-table.tsx
- client/src/app/pages/insights/insight-affected-files-table.tsx
- client/src/app/components/insights/components/insight-expanded-row-content.tsx
- client/src/app/pages/issues/file-incidents-detail-modal/index.ts
- client/src/app/api/rest/analysis.ts
- client/src/app/pages/issues/helpers.ts
- client/src/app/pages/insights/file-incidents-detail-modal/file-all-incidents-table.tsx
- client/src/app/components/insights/tables/use-dynamic-columns.ts
- client/src/app/pages/insights/affected-applications-page.tsx
- client/src/app/pages/insights/insight-detail-drawer.tsx
- client/src/app/components/insights/components/affected-apps-link.tsx
- client/src/app/queries/analysis.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: sjd78
PR: konveyor/tackle2-ui#2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like `getIssue` to `getInsight` and parameters like `issueId` to `insightId` in the analysis API client functions.
client/src/app/components/insights/tables/single-application-insights-table.tsx (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
client/src/app/api/models.ts (1)
Learnt from: sjd78
PR: #2446
File: client/src/app/api/rest/analysis.ts:38-63
Timestamp: 2025-07-08T20:16:53.929Z
Learning: Issue #2445 is tracking the remaining naming inconsistencies from the issues-to-insights refactoring, specifically renaming functions like getIssue
to getInsight
and parameters like issueId
to insightId
in the analysis API client functions.
⏰ 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 (8)
client/src/app/api/models.ts (8)
11-17
: Well-designed generic type for UI table handlingThe
WithUiId<T>
type is a clean solution for adding unique identifiers to objects that lack a single unique key field. The documentation clearly explains its purpose for table row handling.
614-628
: Comprehensive AnalysisInsight interface designThe new
AnalysisInsight
interface properly models the expanded insight data structure with optional incidents, links, and facts. The interface provides good extensibility while maintaining type safety.
630-638
: Well-structured AnalysisIncident interfaceThe
AnalysisIncident
interface appropriately captures incident details including file location, code snippets, and associated facts. The typing is consistent with the overall model design.
647-660
: Clear documentation and consistent naming for report typesThe
AnalysisReportInsight
interface is well-documented with helpful comments explaining the purpose and structure. The naming follows consistent patterns with other report types in the codebase.
662-662
: Appropriate use of WithUiId wrapper typeThe
UiAnalysisReportInsight
type correctly applies theWithUiId
wrapper to provide unique identifiers for UI table usage, following the established pattern.
664-682
: Consistent application-specific insight reporting structureThe
AnalysisReportApplicationInsight
interface maintains consistency with the broader reporting model while providing application-specific insight data. The UI wrapper type is properly defined.
683-704
: Comprehensive insight application reporting modelThe
AnalysisReportInsightApplication
interface provides a complete view of insight data for applications, including business context and nested insight details. The structure supports the UI requirements effectively.
706-713
: Clear file-level insight reporting structureThe
AnalysisReportFile
interface (renamed fromAnalysisFileReport
) provides appropriate file-level insight aggregation with incident counts and effort totals. The renaming improves consistency with other report types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I see from generated code review and basic look I'm going to approve for Scott. LGTM visually
Resolves: #2445
Resolves: https://issues.redhat.com/browse/MTA-5459
Add an Insights page as a clone of the Issues page to shows analysis insights that have an
effort=0
. Insights with aneffort>0
are visible in the existing Issues page.Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Refactor
Chores