refactor(web): use _id for tasks - #1460
Conversation
- Changed all instances of 'id' to '_id' in task-related tests and components to ensure consistency with the new task identifier format. - Updated task creation and manipulation functions to utilize the new '_id' property. - Adjusted related test cases to reflect these changes, enhancing the accuracy of task management functionalities.
…orage utilities - Changed the task identifier from 'id' to '_id' in TaskSchema for consistency. - Updated the saveTaskToIndexedDB and saveTasksToIndexedDB functions to accept the modified Task type. - Removed outdated test case that checked for the presence of '_id' in tasks, streamlining the test suite.
…ests - Added a new utility function `createMockTask` to streamline the creation of task objects in tests, ensuring consistency and reducing redundancy. - Updated existing test files to utilize `createMockTask`, enhancing readability and maintainability of the test suite. - Removed outdated task creation logic from tests, simplifying the code and improving clarity.
There was a problem hiding this comment.
Pull request overview
This PR refactors the task data model to use _id instead of id as the primary identifier for tasks, aligning with MongoDB conventions. A new utility function createMockTask was introduced to streamline test task creation, replacing duplicated task object definitions throughout the test suite.
Changes:
- Renamed task identifier from
idto_idacross the codebase - Added
createMockTaskfactory function for consistent test task creation - Updated all task-related code and tests to use the new
_idproperty
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web/src/common/types/task.types.ts | Updated TaskSchema to use _id field |
| packages/web/src/tests/utils/factories/task.factory.ts | Added new createMockTask factory function |
| packages/web/src/views/Now/shortcuts/useNowShortcuts.test.ts | Refactored to use createMockTask |
| packages/web/src/views/Now/hooks/useFocusedTask.ts | Updated to reference _id |
| packages/web/src/views/Now/hooks/useFocusedTask.test.ts | Refactored to use createMockTask |
| packages/web/src/views/Now/hooks/useAvailableTasks.test.ts | Refactored to use createMockTask |
| packages/web/src/views/Now/context/NowViewProvider.tsx | Updated to reference _id |
| packages/web/src/views/Now/components/TaskSelector/TaskSelector.tsx | Updated to reference _id |
| packages/web/src/views/Now/components/TaskSelector/TaskSelector.test.tsx | Refactored to use createMockTask |
| packages/web/src/views/Now/components/FocusedTask/FocusedTask.test.tsx | Refactored to use createMockTask |
| packages/web/src/views/Now/components/AvailableTasks/AvailableTasks.tsx | Updated to reference _id |
| packages/web/src/views/Now/components/AvailableTasks/AvailableTasks.test.tsx | Refactored to use createMockTask |
| packages/web/src/views/Day/view/DayViewContent.tsx | Updated to reference _id |
| packages/web/src/views/Day/hooks/tasks/useTaskState.test.ts | Refactored to use createMockTask |
| packages/web/src/views/Day/hooks/tasks/useTaskActions.ts | Updated to reference _id throughout |
| packages/web/src/views/Day/hooks/tasks/useTaskActions.test.ts | Refactored to use createMockTask |
| packages/web/src/views/Day/hooks/tasks/useSaveTasksByDateEffect.test.ts | Refactored to use createMockTask |
| packages/web/src/views/Day/hooks/tasks/useLoadTasksByDateEffect.test.ts | Refactored to use createMockTask |
| packages/web/src/views/Day/context/tests/TaskContext.test.tsx | Refactored to use createMockTask |
| packages/web/src/views/Day/context/tests/DNDTasksContext.test.tsx | Updated to reference _id |
| packages/web/src/views/Day/components/Tasks/Tasks.tsx | Updated to reference _id |
| packages/web/src/views/Day/components/Task/Task.tsx | Updated to reference _id |
| packages/web/src/views/Day/components/Task/Task.test.tsx | Refactored to use createMockTask |
| packages/web/src/views/Day/components/Task/DraggableTask.tsx | Updated to reference _id |
| packages/web/src/views/Day/components/Task/DraggableTask.test.tsx | Refactored to use createMockTask |
| packages/web/src/views/Day/components/ContextMenu/TaskContextMenuWrapper.tsx | Updated to reference _id |
| packages/web/src/views/Day/components/ContextMenu/TaskContextMenuItems.tsx | Updated to reference _id |
| packages/web/src/index.tsx | Added error logging for database initialization |
| packages/web/src/common/utils/task/sort.task.test.ts | Refactored to use createMockTask |
| packages/web/src/common/utils/storage/task.storage.util.ts | Updated to reference _id and removed validation check |
| packages/web/src/common/utils/storage/task.storage.util.test.ts | Refactored to use createMockTask |
| packages/web/src/common/utils/storage/task-migration.util.test.ts | Refactored to use createMockTask |
| packages/web/src/common/utils/storage/storage.util.ts | Updated to reference _id |
| packages/web/src/common/utils/storage/compass-local.db.ts | Updated IndexedDB schema to use _id |
| packages/web/src/common/services/task/task-storage-migration.service.test.ts | Refactored to use createMockTask |
| packages/web/src/common/repositories/task/local.task.repository.ts | Updated to reference _id |
| packages/web/src/common/repositories/task/local.task.repository.test.ts | Updated to reference _id |
| packages/web/src/common/repositories/task/cloud.task.repository.test.ts | Refactored to use createMockTask |
| packages/web/src/tests/utils/repositories/repository.test.factory.ts | Updated createTestTask to use _id |
…aration - Eliminated the onboarding state initialization from the `prepareTaskPage` function in task-test-utils, simplifying the test setup. - Updated log messages in the delete command to clarify the contents of LocalStorage and IndexedDB after account deletion. - Enhanced error message styling in the CleanupView component for better visibility.
- Refactored task storage functions to utilize IndexedDB for loading and saving tasks, improving performance and data persistence. - Updated related functions to handle asynchronous operations, ensuring compatibility with the new storage methods. - Added event dispatching for task save actions to support synchronization across tabs. - Enhanced error handling for storage operations, providing clearer logging for debugging. - Updated tests to reflect changes in asynchronous behavior and ensure reliability of task management functionalities.
dbd2994 to
e0ba475
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 46 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (3)
packages/web/src/common/utils/storage/task-migration.util.test.ts:6
- This test suite now only migrates tasks created by the new
createMockTaskfactory (which already uses_id). Since production localStorage data may still contain legacy tasks withid, please add/keep a test case that stores a legacy{ id: ... }task in localStorage and asserts it is migrated/normalized to_idcorrectly (and doesn’t incorrectly set the migration-complete flag).
import { createMockTask } from "@web/__tests__/utils/factories/task.factory";
import {
hasTaskMigrationCompleted,
migrateTasksFromLocalStorageToIndexedDB,
resetTaskMigrationFlag,
} from "./task-migration.util";
packages/web/src/common/utils/storage/task-migration.util.ts:40
- The migration logic relies on
isTaskvalidation, butTaskSchemanow requires_id, so legacy tasks in localStorage that still useidwill be skipped and may never be migrated. Please make the migration backwards-compatible (acceptidand normalize to_id) and ensure the completion flag isn’t set unless legacy keys have actually been migrated/cleared.
packages/web/src/common/utils/storage/task-migration.util.ts:38 - Leftover
console.logcalls in a migration path will add noise in production logs and can leak implementation details. Please remove these logs or gate them behind a proper debug/logger mechanism.
…ove error handling
- Added functionality to migrate tasks from legacy storage to IndexedDB, including reading legacy data and normalizing tasks. - Introduced new utility functions for handling IndexedDB operations, improving database interaction and error handling. - Updated the task migration process to ensure seamless transition and data integrity during migration. - Enhanced existing task migration logic to accommodate legacy task structures and ensure compatibility with the new storage system.
…sk synchronization logic - Eliminated the storage event listener for cross-tab synchronization, simplifying the task reload mechanism. - Removed unnecessary console log from the task saved event handler, enhancing code clarity.
| import { Task } from "@web/common/types/task.types"; | ||
| import { createObjectIdString } from "@web/common/utils/id/object-id.util"; | ||
|
|
||
| export const createMockTask = (overrides?: Partial<Task>): Task => ({ | ||
| _id: createObjectIdString(), | ||
| title: "Test Task", | ||
| status: "todo", | ||
| order: 0, | ||
| createdAt: new Date().toISOString(), | ||
| user: "user-1", | ||
| ...overrides, | ||
| }); |
There was a problem hiding this comment.
The PR description doesn't accurately reflect the scope of changes. It only mentions adding a createMockTask utility and updating tests, but this PR actually includes a major refactoring:
- Renamed the
idproperty to_idin the Task type - Updated the IndexedDB schema to use
_idas the key path - Converted storage utilities from synchronous (localStorage) to asynchronous (IndexedDB)
- Added migration logic to handle legacy tasks with the
idfield - Updated all components, hooks, and tests to use
_id
Consider updating the PR description to accurately reflect these significant architectural changes.
createMockTaskto streamline the creation of task objects in tests, ensuring consistency and reducing redundancy.createMockTask, enhancing readability and maintainability of the test suite.Closes #1459