feat(auth): implement session user synchronization and related tests#1469
feat(auth): implement session user synchronization and related tests#1469tyler-dane wants to merge 1 commit intomainfrom
Conversation
- Added `syncLocalTaskUsersToAuthenticatedUser` utility to synchronize task users with the authenticated user upon login. - Updated `SessionProvider` to trigger synchronization when the authentication state changes. - Introduced tests for `SessionProvider` to verify synchronization behavior based on authentication status. - Enhanced `LocalTaskRepository` to normalize unauthenticated users when saving tasks. - Added comprehensive tests for the synchronization utility to ensure correct behavior across various scenarios.
There was a problem hiding this comment.
Pull request overview
This PR implements a synchronization mechanism to update local task user fields from unauthenticated placeholders to authenticated user IDs upon login. The synchronization ensures that tasks created before authentication are properly associated with the authenticated user.
Changes:
- Added utility function to sync unauthenticated task users to authenticated user across all dates
- Integrated synchronization into SessionProvider to trigger automatically upon authentication
- Enhanced LocalTaskRepository to normalize task users during save operations
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/web/src/common/utils/sync/local-task-user-sync.util.ts |
New utility function that scans all tasks and updates those with UNAUTHENTICATED_USER to the authenticated user ID |
packages/web/src/common/utils/sync/local-task-user-sync.util.test.ts |
Comprehensive test coverage for the sync utility including edge cases and error scenarios |
packages/web/src/common/repositories/task/local.task.repository.ts |
Added normalizeUnauthenticatedUsers method to update task users during save operations |
packages/web/src/common/repositories/task/local.task.repository.test.ts |
Tests verifying normalization behavior for authenticated and unauthenticated states |
packages/web/src/auth/session/SessionProvider.tsx |
Integrated sync function into useEffect hook to run when authentication state becomes true |
packages/web/src/auth/session/SessionProvider.test.tsx |
Tests verifying sync is triggered on authentication and not triggered when unauthenticated |
| fireEvent.click( | ||
| screen.getByRole("button", { name: "Set Unauthenticated" }), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Missing test coverage for error handling when syncLocalTaskUsersToAuthenticatedUser fails. While the SessionProvider correctly catches and logs errors from the sync function, there should be a test verifying that errors are properly handled and don't crash the component. This would ensure the error handling path is tested and maintainable.
| }); | |
| }); | |
| it("handles errors from syncing task users without crashing", async () => { | |
| const consoleErrorSpy = jest | |
| .spyOn(console, "error") | |
| .mockImplementation(() => {}); | |
| mockSyncLocalTaskUsersToAuthenticatedUser.mockRejectedValueOnce( | |
| new Error("sync failed"), | |
| ); | |
| render( | |
| <SessionProvider> | |
| <SessionTestHarness /> | |
| </SessionProvider>, | |
| ); | |
| fireEvent.click(screen.getByRole("button", { name: "Set Authenticated" })); | |
| await waitFor(() => { | |
| expect(mockSyncLocalTaskUsersToAuthenticatedUser).toHaveBeenCalledTimes( | |
| 1, | |
| ); | |
| }); | |
| // The component should still be rendered and interactive despite the error. | |
| expect( | |
| screen.getByRole("button", { name: "Set Authenticated" }), | |
| ).toBeInTheDocument(); | |
| expect(consoleErrorSpy).toHaveBeenCalled(); | |
| consoleErrorSpy.mockRestore(); | |
| }); |
| user: UNAUTHENTICATED_USER, | ||
| }), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Missing test coverage for error handling when getUserId fails during save. While the normalizeUnauthenticatedUsers method correctly catches and logs errors from getUserId, there should be a test verifying this error path to ensure tasks are still saved with their original user field when getUserId fails.
| }); | |
| }); | |
| it("keeps original task user when getUserId fails", async () => { | |
| const dateKey = "2024-01-01"; | |
| const originalUser = UNAUTHENTICATED_USER; | |
| mockGetUserId.mockRejectedValue(new Error("getUserId failed")); | |
| const task = createTestTask({ | |
| _id: "task-1", | |
| title: "Task One", | |
| user: originalUser, | |
| }); | |
| await repository.save(dateKey, task); | |
| expect(mockAdapter.putTask).toHaveBeenCalledWith( | |
| dateKey, | |
| expect.objectContaining({ | |
| _id: "task-1", | |
| user: originalUser, | |
| }), | |
| ); | |
| }); |
|
Closing as unnecessary for now. We can follow the same pattern that we're using for events for now: When a user connects their GCal, we convert IDB events to cloud events. In other words, they're deleted from IDB. This is a simple way to move forward. |
syncLocalTaskUsersToAuthenticatedUserutility to synchronize task users with the authenticated user upon login.SessionProviderto trigger synchronization when the authentication state changes.SessionProviderto verify synchronization behavior based on authentication status.LocalTaskRepositoryto normalize unauthenticated users when saving tasks.Closes Tasks still store
"UNAUTHENTICATED_USER"after connecting gcal #1467