✨ Refactor analysis API and update query hooks #2481
Conversation
This commit introduces a comprehensive refactor of the analysis API, consolidating various report fetching functions and updating the corresponding hooks in the application. The following changes were made: - Replaced old report fetching functions with new ones in `analysis.ts`. - Updated components to use the new hooks for fetching application issues, insights, and incidents. - Removed deprecated query hooks from `issues.ts` to streamline the codebase. Part-of: konveyor#2445 Signed-off-by: Scott J Dickerson <[email protected]>
WalkthroughThis change refactors the analysis-related API and query logic in the client application. It separates the concepts of "insights" and "issues," updates endpoint constants, renames and restructures related functions and hooks, and migrates all usage in UI components to the new structure. The previous queries module is removed and replaced. Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant AnalysisQueries
participant RESTClient
UI->>AnalysisQueries: useFetchReportAllIssues()
AnalysisQueries->>RESTClient: getReportAllIssues()
RESTClient-->>AnalysisQueries: Issue data (effort>0)
AnalysisQueries-->>UI: { data, total, isFetching, ... }
UI->>AnalysisQueries: useFetchReportAllInsights()
AnalysisQueries->>RESTClient: getReportAllInsights()
RESTClient-->>AnalysisQueries: Insight data (effort=0)
AnalysisQueries-->>UI: { data, total, isFetching, ... }
Estimated code review effort3 (~45 minutes) Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
client/src/app/queries/analysis.ts (3)
186-186: Misleading comment about hook being generic.The comment states "Generic for issues and insights" but the function is specifically named
useFetchInsight. This is misleading since it only fetches insights, not issues.-// Generic for issues and insights +// Fetch individual insight by ID
196-196: Consider improving error handling beyond console logging.All three hooks use
onError: (error) => console.log("error, ", error)which may not provide adequate error visibility in production environments.Consider using a more robust error handling approach:
onError: (error) => { console.error(`Failed to fetch ${queryKey}:`, error); // Consider adding error reporting service here },Also applies to: 219-219, 242-242
74-84: Inconsistent return type structures across hooks.The helper functions return different data structures - some use
HubPaginatedResult<WithUiId<T>>with UI IDs added, while others return simpler objects without UI ID transformation.Consider standardizing the return type structure across all hooks for consistency, or document the reasoning for the different approaches.
Also applies to: 134-144, 179-184
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
client/src/app/api/rest/analysis.ts(1 hunks)client/src/app/pages/issues/affected-applications/affected-applications.tsx(2 hunks)client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-all-incidents-table.tsx(1 hunks)client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-incidents-detail-modal.tsx(1 hunks)client/src/app/pages/issues/issue-detail-drawer/issue-affected-files-table.tsx(2 hunks)client/src/app/pages/issues/issue-detail-drawer/issue-detail-drawer.tsx(2 hunks)client/src/app/pages/issues/issues-table.tsx(2 hunks)client/src/app/queries/analysis.ts(1 hunks)client/src/app/queries/issues.ts(0 hunks)
🧠 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/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/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/pages/issues/issue-detail-drawer/issue-detail-drawer.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/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.
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.
🧬 Code Graph Analysis (5)
client/src/app/pages/issues/issue-detail-drawer/issue-affected-files-table.tsx (1)
client/src/app/queries/analysis.ts (1)
useFetchReportInsightFiles(230-251)
client/src/app/pages/issues/issues-table.tsx (1)
client/src/app/queries/analysis.ts (2)
useFetchReportApplicationIssues(86-98)useFetchReportAllIssues(32-44)
client/src/app/pages/issues/issue-detail-drawer/issue-detail-drawer.tsx (1)
client/src/app/queries/analysis.ts (1)
useFetchInsight(187-205)
client/src/app/queries/analysis.ts (3)
client/src/app/api/models.ts (3)
HubRequestParams(23-33)HubPaginatedResult(35-39)WithUiId(662-662)client/src/app/api/rest/analysis.ts (9)
getReportAllIssues(59-60)getReportAllInsights(75-76)getReportApplicationIssues(62-69)getReportApplicationInsights(78-85)getReportIssueApps(71-72)getReportInsightApps(87-88)getInsight(36-39)getInsightIncidents(41-50)getInsightFiles(52-56)client/src/app/utils/query-utils.ts (1)
useWithUiId(13-33)
client/src/app/pages/issues/affected-applications/affected-applications.tsx (1)
client/src/app/queries/analysis.ts (1)
useFetchReportIssueApps(146-155)
💤 Files with no reviewable changes (1)
- client/src/app/queries/issues.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/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/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/pages/issues/issue-detail-drawer/issue-detail-drawer.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/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.
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.
🧬 Code Graph Analysis (5)
client/src/app/pages/issues/issue-detail-drawer/issue-affected-files-table.tsx (1)
client/src/app/queries/analysis.ts (1)
useFetchReportInsightFiles(230-251)
client/src/app/pages/issues/issues-table.tsx (1)
client/src/app/queries/analysis.ts (2)
useFetchReportApplicationIssues(86-98)useFetchReportAllIssues(32-44)
client/src/app/pages/issues/issue-detail-drawer/issue-detail-drawer.tsx (1)
client/src/app/queries/analysis.ts (1)
useFetchInsight(187-205)
client/src/app/queries/analysis.ts (3)
client/src/app/api/models.ts (3)
HubRequestParams(23-33)HubPaginatedResult(35-39)WithUiId(662-662)client/src/app/api/rest/analysis.ts (9)
getReportAllIssues(59-60)getReportAllInsights(75-76)getReportApplicationIssues(62-69)getReportApplicationInsights(78-85)getReportIssueApps(71-72)getReportInsightApps(87-88)getInsight(36-39)getInsightIncidents(41-50)getInsightFiles(52-56)client/src/app/utils/query-utils.ts (1)
useWithUiId(13-33)
client/src/app/pages/issues/affected-applications/affected-applications.tsx (1)
client/src/app/queries/analysis.ts (1)
useFetchReportIssueApps(146-155)
🔇 Additional comments (14)
client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-all-incidents-table.tsx (1)
6-6: LGTM! Import migration aligns with refactoring goals.The import update from the deprecated
@app/queries/issuesto the new@app/queries/analysismodule is consistent with the broader refactoring effort. The hook usage remains unchanged, ensuring functional compatibility.client/src/app/pages/issues/issue-detail-drawer/file-incidents-detail-modal/file-incidents-detail-modal.tsx (1)
21-21: LGTM! Consistent import migration.The import update follows the same pattern as other files in this refactor, moving from the deprecated
@app/queries/issuesto@app/queries/analysiswhile maintaining the same hook interface.client/src/app/pages/issues/affected-applications/affected-applications.tsx (2)
29-29: LGTM! Hook name updated to reflect new API structure.The import correctly updates to use
useFetchReportIssueAppsfrom the new analysis module, which better reflects the separation between issues and insights in the refactored API.
79-101: LGTM! Hook usage updated consistently.The hook usage is correctly updated to
useFetchReportIssueAppswith the same parameters, maintaining functional compatibility while adopting the new API structure. The implicit filters and sorting configuration remain properly configured.client/src/app/pages/issues/issue-detail-drawer/issue-affected-files-table.tsx (2)
17-17: LGTM! Import updated to use insight-focused API.The import correctly updates to
useFetchReportInsightFilesfrom the analysis module, reflecting the new API structure that separates insights and issues.
71-81: LGTM! Hook usage updated with correct parameters.The hook usage is properly updated to
useFetchReportInsightFileswith the same parameter structure. Theissue.idparameter correctly corresponds to theinsightIdexpected by the new API. This aligns with the broader refactoring tracked in issue #2445.client/src/app/pages/issues/issues-table.tsx (2)
37-40: LGTM! Imports updated to use more specific hooks.The imports correctly replace the generic hooks with more specific ones:
useFetchReportApplicationIssuesfor single app mode anduseFetchReportAllIssuesfor all issues mode. This better reflects the API's separation of concerns.
200-207: LGTM! Hook usage correctly adapted for different modes.The hook usage is properly updated:
useFetchReportApplicationIssuescorrectly passesselectedAppIdfor single app modeuseFetchReportAllIssuescorrectly passes the enabled flag based on mode- Both hooks receive the same
hubRequestParams, maintaining consistent parameter handlingThe conditional logic appropriately selects the right query based on the component mode.
client/src/app/pages/issues/issue-detail-drawer/issue-detail-drawer.tsx (1)
37-37: Hook usage correctly handles optional parameter.The usage of
useFetchInsight(issueId || undefined)correctly handles the optional parameter requirement of the new hook, ensuring the query is only enabled when an ID is provided.client/src/app/queries/analysis.ts (2)
20-30: Well-organized query keys with clear naming conventions.The query keys are consistently named with descriptive prefixes that clearly distinguish between different data types (issues, insights, applications).
60-84: Excellent use of generic helper function to reduce code duplication.The
useFetchReportAll_helper function effectively eliminates code duplication betweenuseFetchReportAllIssuesanduseFetchReportAllInsightswhile maintaining type safety.client/src/app/api/rest/analysis.ts (3)
14-22: Excellent domain documentation.The comprehensive comment block clearly explains the relationship between analyses, rulesets, rules, insights, issues, and incidents. This greatly improves code maintainability and understanding.
28-34: Clear separation of issues and insights with proper filtering.The URL constants properly distinguish between issues (effort>0) and insights (effort=0) using appropriate query filters, making the API endpoints semantically clear.
58-88: Well-structured separation of issue and insight functions.The clear separation between issue-specific functions (lines 58-72) and insight-specific functions (lines 74-88) with consistent naming patterns improves code organization and aligns with the domain model described in the comments.
Signed-off-by: Scott J Dickerson <[email protected]>
This commit introduces a comprehensive refactor of the analysis API, consolidating various report fetching functions and updating the corresponding hooks in the application. The following changes were made:
analysis.ts.issues.tsto streamline the codebase.Part-of: #2445
Summary by CodeRabbit
New Features
Refactor