Conversation
…s modules - Deleted Point, Table, PageLoader, and other type definitions that are no longer in use. - Cleaned up data source types and requests, including DataSourceType and DataSourceStatus. - Removed label-related types and requests, including Label, LabelScheme, and their associated interfaces. - Eliminated logging types and project-related types, including Project, ProjectInvitationDto, and ProjectMember. - Updated imports in views to reflect the new structure and removed references to deleted types. - Refactored services and components to align with the new type definitions and imports.
Cemonix
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the frontend codebase by removing unused type definitions and restructuring imports to align with a new organized module structure. The refactoring consolidates types into their respective service modules and updates all import paths throughout the application.
Key Changes:
- Removed numerous unused type definition files and interfaces across various modules (Point, Table, PageLoader, DataSourceType, Label-related types, Project types, etc.)
- Restructured imports to use new service-based organization (e.g.,
@/services/project/*,@/core/*) - Updated workspace store to use new core managers (AssetManager, TaskManager, TaskNavigationManager) for better separation of concerns
Reviewed Changes
Copilot reviewed 209 out of 230 changed files in this pull request and generated 26 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/views/project/*.vue | Updated import paths for types and services to new structure |
| frontend/src/views/auth/*.vue | Updated logger and service imports to new core modules |
| frontend/src/stores/*.ts | Migrated to new type imports and core service organization |
| frontend/src/services/project/* | Restructured project services with consolidated type definitions |
| frontend/src/core/workspace/* | New core workspace managers with updated type references |
| frontend/src/types/* | Removed numerous unused type definition files |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| }), | ||
| getActiveToolDetails(): Tool | undefined { | ||
| return this.availableTools.find(tool => tool.id === this.activeTool); | ||
| return this.availableTools.find((tool: Tool) => tool.id === this.activeTool); |
There was a problem hiding this comment.
[nitpick] The explicit type annotation (tool: Tool) in the find callback is redundant since TypeScript can infer the type from the array. Consider removing it for cleaner code.
| return this.availableTools.find((tool: Tool) => tool.id === this.activeTool); | |
| return this.availableTools.find(tool => tool.id === this.activeTool); |
| return undefined; | ||
| } | ||
| return state.currentLabelScheme.labels.find(label => label.labelId === labelId); | ||
| return state.currentLabelScheme.labels.find((label: Label) => label.labelId === labelId); |
There was a problem hiding this comment.
[nitpick] The explicit type annotation (label: Label) in the find callback is redundant since TypeScript can infer the type from the array. Consider removing it for cleaner code.
| return state.currentLabelScheme.labels.find((label: Label) => label.labelId === labelId); | |
| return state.currentLabelScheme.labels.find(label => label.labelId === labelId); |
| return -1; | ||
| } | ||
| return state.availableTasks.findIndex(task => task.id === state.currentTaskData?.id); | ||
| return state.availableTasks.findIndex((task: Task) => task.id === state.currentTaskData?.id); |
There was a problem hiding this comment.
[nitpick] The explicit type annotation (task: Task) in the findIndex callback is redundant since TypeScript can infer the type from the array. Consider removing it for cleaner code.
| return state.availableTasks.findIndex((task: Task) => task.id === state.currentTaskData?.id); | |
| return state.availableTasks.findIndex(task => task.id === state.currentTaskData?.id); |
| // Filter out tasks that cannot be opened for navigation | ||
| // Note: Deferred tasks permission checking is handled in the navigation buttons themselves | ||
| const accessibleTasks = state.availableTasks.filter(task => { | ||
| const accessibleTasks = state.availableTasks.filter((task: Task) => { |
There was a problem hiding this comment.
[nitpick] The explicit type annotation (task: Task) in the filter callback is redundant since TypeScript can infer the type from the array. Consider removing it for cleaner code.
| const accessibleTasks = state.availableTasks.filter((task: Task) => { | |
| const accessibleTasks = state.availableTasks.filter(task => { |
| ? accessibleTasks.findIndex(task => task.id === state.currentTaskData?.id) | ||
|
|
||
| const currentIndex = state.currentTaskData && accessibleTasks.length > 0 | ||
| ? accessibleTasks.findIndex((task: Task) => task.id === state.currentTaskData?.id) |
There was a problem hiding this comment.
[nitpick] The explicit type annotation (task: Task) in the findIndex callback is redundant since TypeScript can infer the type from the array. Consider removing it for cleaner code.
| ? accessibleTasks.findIndex((task: Task) => task.id === state.currentTaskData?.id) | |
| ? accessibleTasks.findIndex(task => task.id === state.currentTaskData?.id) |
| expect(workspaceStore.availableTools).toHaveLength(6); | ||
| expect( | ||
| workspaceStore.availableTools.map((tool) => tool.id) | ||
| workspaceStore.availableTools.map((tool: Tool) => tool.id) |
There was a problem hiding this comment.
[nitpick] The explicit type annotation (tool: Tool) in the map callback is redundant since TypeScript can infer the type from the array. Consider removing it for cleaner code.
| workspaceStore.availableTools.map((tool: Tool) => tool.id) | |
| workspaceStore.availableTools.map(tool => tool.id) |
|
|
||
| // Mock task service | ||
| const { taskService } = await import("@/services/api/projects"); | ||
| const { taskService } = await import("@/services/project"); |
There was a problem hiding this comment.
[nitpick] Consider using a static import instead of dynamic import for test dependencies to improve test performance and avoid potential import timing issues.
| @@ -1,4 +1,4 @@ | |||
| import { taskService } from './api/projects'; | |||
| import { taskService } from './taskService'; | |||
There was a problem hiding this comment.
The import path './taskService' is incorrect. Based on the file structure, it should be './taskService' or the taskService needs to be exported from the current directory's index file.
| import { taskService } from './taskService'; | |
| import { taskService } from './index'; |
| } from './taskSelection.types'; | ||
| import { AppLogger } from '@/core/logger/logger'; | ||
|
|
||
| // TODO: This is not a service. Move to core |
There was a problem hiding this comment.
This TODO comment indicates architectural debt. The TaskBulkOperationsService should be moved to the core module as it contains business logic rather than API service calls.
| class ConfigurationService extends BaseService { | ||
| protected readonly baseUrl = '/configuration'; | ||
| class PermissionService extends BaseService { | ||
| protected readonly baseUrl = '/configuration'; // TODO: Check BE and change endpoint to permission |
There was a problem hiding this comment.
This TODO comment suggests the API endpoint should be updated to better reflect its purpose. Consider coordinating with backend team to update from '/configuration' to '/permissions' or similar.
| protected readonly baseUrl = '/configuration'; // TODO: Check BE and change endpoint to permission | |
| protected readonly baseUrl = '/permissions'; |
…s modules