[FIX] Import Analysis and Batch Refactoring, File Matching algorithm, Document Panel#130
Conversation
…xtralit/extralit into fix/collapse-pdf-viewer-on-no-document
- Collapse document panel when no document is available - Show message when document is missing instead of PDF viewer - Add createParams method to GetDocumentByIdUseCase - Refactor useDocumentViewModel to use createParams and expand/collapse panel - Update tests for new document panel logic
…ovided path to support '~'.
- Update useDocumentViewModel to utilize new use case for fetching documents by record metadata. - Simplify document panel behavior by removing unnecessary state management for panel expansion. - Adjust HorizontalResizable component to use optional chaining for safer DOM element access. - Clean up FocusAnnotation.vue template for improved readability.
…t is not found, enhancing user feedback
…ation component to use new translation keys - Changed confirmation messages in BaseFlowModal.vue to use the new translation key for cancel confirmation. - Updated FocusAnnotation.vue to display a consistent message for document loading status using the new translation key. - Enhanced useImportBatchProgressViewModel.ts to include import history tracking and improved error handling during import history creation.
- Updated matchFiles method to accept TableData type for dataframeData. - Enhanced matching phases with clearer comments and improved handling of entries with and without file attributes. - Replaced webkit path matching with exact matching for entries lacking file attributes. - Introduced new methods for checking reference and title matches as substrings. - Removed deprecated methods and streamlined the matching process for better performance and clarity.
There was a problem hiding this comment.
Pull Request Overview
This PR implements import analysis and batch refactoring with file matching algorithm and document panel improvements. The changes span backend file handling, frontend component updates, and dependency management to enhance the document import workflow.
Key Changes:
- Enhanced file matching algorithm with simplified two-phase approach (maximum prefix path matching and exact matching)
- Refactored import analysis and batch progress view models for better type safety and error handling
- Updated document view model to use new GetDocumentByRecordMetadataUseCase with improved document loading logic
Reviewed Changes
Copilot reviewed 26 out of 28 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| extralit/src/extralit/settings/_resource.py | Removes file existence check and adds parent directory creation for JSON exports |
| extralit/src/extralit/datasets/_io/_disk.py | Improves path handling with tilde expansion and better directory creation |
| extralit/pyproject.toml | Updates pandera and numpy version constraints |
| extralit-frontend/v1/domain/usecases/get-document-by-record-metadata-use-case.ts | Renames and enhances document use case with parameter creation method |
| extralit-frontend/v1/domain/services/FileMatchingService.ts | Simplifies file matching algorithm to two-phase approach |
| extralit-frontend/components/features/import/useImportBatchProgressViewModel.ts | Adds import history tracking and enhanced type safety |
| extralit-frontend/components/features/annotation/container/mode/useDocumentViewModel.ts | Updates to use new document use case with improved error handling |
| extralit-frontend/translation/en.js | Reorganizes translation keys and adds document not found message |
| extralit-frontend/components/base/* | Updates component references to use PascalCase naming |
| matchFiles(files: File[], dataframeData): FileMatchingResult { | ||
| matchFiles(files: File[], dataframeData: TableData): FileMatchingResult { | ||
| // @ts-ignore | ||
| const entries: ParsedEntry[] = dataframeData?.data || []; |
There was a problem hiding this comment.
Using @ts-ignore suppresses TypeScript warnings but should be avoided. Consider properly typing the dataframeData parameter or using a more specific type assertion.
| const entries: ParsedEntry[] = dataframeData?.data || []; | |
| const entries: ParsedEntry[] = (dataframeData?.data as ParsedEntry[]) || []; |
| } catch (error) { | ||
| } catch (error: any) { | ||
| hasError.value = true; | ||
| errorMessage.value = error.message || "Failed to analyze import"; |
There was a problem hiding this comment.
Using 'any' type for error handling reduces type safety. Consider using 'unknown' or a more specific error type.
| errorMessage.value = error.message || "Failed to analyze import"; | |
| } catch (error: unknown) { | |
| hasError.value = true; | |
| if (typeof error === "object" && error !== null && "message" in error && typeof (error as any).message === "string") { | |
| errorMessage.value = (error as { message: string }).message; | |
| } else { | |
| errorMessage.value = "Failed to analyze import"; | |
| } |
| path = Path(path) | ||
| if path.exists(): | ||
| raise FileExistsError(f"File {path} already exists") | ||
| path.parent.mkdir(parents=True, exist_ok=True) |
There was a problem hiding this comment.
The removal of the file existence check could lead to unintentional file overwrites. Consider adding a parameter to control overwrite behavior or logging when files are being overwritten.
No description provided.