:chores: Moved dataset import/export to its own feature folder #249
Conversation
jpggvilaca
requested review from
MarkRedeman,
camiloHimura,
pplaskie and
ActiveChooN
as code owners
jpggvilaca
force-pushed
the
jvilaca/chores-dataset-import-feature
branch
from
192a253 to
94db4d3
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the codebase by consolidating all dataset export functionality into a new features/dataset-export folder, updating import paths and module references accordingly. It centralizes export-related components, hooks, and utilities under one feature, and renames CSS modules to dataset-export.module.scss.
- Updated import paths in tests and components to reflect new feature directory structure
- Renamed styling modules from
project-dataset.module.scsstodataset-export.module.scssand removed an unsafe className on a divider - Moved
IntervalJobHandlerstype import from core datasets to feature dataset-import, creating a cross-feature dependency
Reviewed Changes
Copilot reviewed 75 out of 76 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| web_ui/src/features/dataset-export/utils/utils.test.ts | Adjusted import path for ExportFormats |
| web_ui/src/features/dataset-export/hooks/use-export-dataset.hook.ts | Updated IntervalJobHandlers import location |
| web_ui/src/features/dataset-export/components/export-import-dataset-dialog-provider.component.tsx | Adjusted provider import path |
| web_ui/src/features/dataset-export/components/export-import-dataset-buttons.test.tsx | Updated test imports and mocks paths |
| web_ui/src/features/dataset-export/components/export-import-dataset-buttons.component.tsx | Reordered imports and updated paths |
| web_ui/src/features/dataset-export/components/export-dataset-status-job.test.tsx | Updated import paths in tests |
| web_ui/src/features/dataset-export/components/export-dataset-status-job.component.tsx | Updated imports and CSS module reference |
| web_ui/src/features/dataset-export/components/export-dataset-notification.test.tsx | Adjusted import paths in tests |
| web_ui/src/features/dataset-export/components/export-dataset-notification.component.tsx | Updated import paths |
| web_ui/src/features/dataset-export/components/export-dataset-message.test.tsx | Fixed relative import for utils |
| web_ui/src/features/dataset-export/components/export-dataset-message.component.tsx | Updated imports and CSS module reference |
| web_ui/src/features/dataset-export/components/export-dataset-download.test.tsx | Adjusted import paths and mocks |
| web_ui/src/features/dataset-export/components/export-dataset-download.component.tsx | Updated imports and CSS module reference |
| web_ui/src/features/dataset-export/components/export-dataset-dialog.test.tsx | Updated test imports and mocks |
| web_ui/src/features/dataset-export/components/export-dataset-dialog.component.tsx | Adjusted imports, removed unsafe className on Divider |
| web_ui/src/features/dataset-export/components/dataset-statistics.test.tsx | Updated import paths in tests |
| web_ui/src/core/datasets/hooks/utils.ts | Changed IntervalJobHandlers import to feature |
Files not reviewed (1)
- web_ui/src/features/dataset-export/components/dataset-export.module.scss: Language not supported
| @@ -185,7 +185,7 @@ export const ExportDatasetDialog = ({ triggerState, datasetName }: ExportDataset | |||
| Convert frames to individual images | |||
| </Radio> | |||
| </RadioGroup> | |||
| <Divider size='S' UNSAFE_className={classes.divider} /> | |||
| <Divider size='S' /> | |||
There was a problem hiding this comment.
Removing the UNSAFE_className={classes.divider} likely drops intended styling for the divider. Consider re-adding the CSS class or moving styling to a safer prop if needed.
| <Divider size='S' /> | |
| <Divider size='S' UNSAFE_className={classes.divider} /> |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
divider class styles did not exist in the scss module.
| @@ -5,6 +5,7 @@ import { AxiosError } from 'axios'; | |||
| import { StatusCodes } from 'http-status-codes'; | |||
| import { isFunction } from 'lodash-es'; | |||
|
|
|||
| import { IntervalJobHandlers } from '../../../features/dataset-import/types/dataset-import.interface'; | |||
There was a problem hiding this comment.
[nitpick] Core utilities should not depend on feature-level types to avoid cyclic or inverted dependencies. Consider moving IntervalJobHandlers to a shared core or common package.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
+1. We don't want to import anything from features/pages to core.
| import { getIntervalJobHandlers } from '../../../core/datasets/hooks/utils'; | ||
| import { JobExportStatus, JobStatusIdentifier } from '../../../core/jobs/jobs.interface'; | ||
| import { ExportDatasetIdentifier, ExportDatasetStatusIdentifier } from '../../../core/projects/dataset.interface'; | ||
| import { isStateDone, isStateError } from '../../../core/projects/hooks/utils'; | ||
| import QUERY_KEYS from '../../../core/requests/query-keys'; | ||
| import { useApplicationServices } from '../../../core/services/application-services-provider.component'; | ||
| import { IntervalJobHandlers } from '../../../features/dataset-import/types/dataset-import.interface'; |
There was a problem hiding this comment.
[nitpick] Importing IntervalJobHandlers from the dataset-import feature couples export logic to import-specific types. It may be cleaner to define a shared type in core or a common folder.
Copilot uses AI. Check for mistakes.
|
📝 Description
Before:
pages/landing-page/landing-page-workspace/components/dataset-import-to-new-projectshared/components/dataset-import-deletion-dialogproject-details/components/project-dataset/dataset-imports...other scattered folders
After:
OPENS:
✨ Type of Change
Select the type of change your PR introduces:
🧪 Testing Scenarios
Describe how the changes were tested and how reviewers can test them too:
✅ Checklist
Before submitting the PR, ensure the following: