diff --git a/app/src/api/file.ts b/app/src/api/file.ts index 08f4c4f..c85d3a8 100644 --- a/app/src/api/file.ts +++ b/app/src/api/file.ts @@ -40,6 +40,10 @@ export const lookupFileByName = async ( workspaceName: string, filename: string ): Promise => { + if (!filename || typeof filename !== 'string') { + throw new Error('Invalid filename provided for lookup'); + } + const response = await apiCall( `${API_BASE_URL}/workspaces/${encodeURIComponent( workspaceName diff --git a/app/src/hooks/useGitOperations.test.ts b/app/src/hooks/useGitOperations.test.ts new file mode 100644 index 0000000..2cd439a --- /dev/null +++ b/app/src/hooks/useGitOperations.test.ts @@ -0,0 +1,431 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { renderHook, act } from '@testing-library/react'; +import { useGitOperations } from './useGitOperations'; +import * as gitApi from '@/api/git'; + +// Mock dependencies +vi.mock('@/api/git'); +vi.mock('@mantine/notifications', () => ({ + notifications: { + show: vi.fn(), + }, +})); + +// Mock the workspace context +const mockWorkspaceData: { + currentWorkspace: { id: number; name: string } | null; + settings: { gitEnabled: boolean }; +} = { + currentWorkspace: { + id: 1, + name: 'test-workspace', + }, + settings: { + gitEnabled: true, + }, +}; + +vi.mock('../contexts/WorkspaceDataContext', () => ({ + useWorkspaceData: () => mockWorkspaceData, +})); + +// Import notifications for assertions +import { notifications } from '@mantine/notifications'; + +describe('useGitOperations', () => { + beforeEach(() => { + vi.clearAllMocks(); + // Reset workspace data to defaults + mockWorkspaceData.currentWorkspace = { + id: 1, + name: 'test-workspace', + }; + mockWorkspaceData.settings = { + gitEnabled: true, + }; + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + describe('handlePull', () => { + it('pulls changes successfully and shows success notification', async () => { + const mockPullChanges = vi.mocked(gitApi.pullChanges); + mockPullChanges.mockResolvedValue('Successfully pulled latest changes'); + + const { result } = renderHook(() => useGitOperations()); + + let pullResult: boolean | undefined; + await act(async () => { + pullResult = await result.current.handlePull(); + }); + + expect(pullResult).toBe(true); + expect(mockPullChanges).toHaveBeenCalledWith('test-workspace'); + expect(notifications.show).toHaveBeenCalledWith({ + title: 'Success', + message: 'Successfully pulled latest changes', + color: 'green', + }); + }); + + it('handles pull errors and shows error notification', async () => { + const mockPullChanges = vi.mocked(gitApi.pullChanges); + const consoleSpy = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); + + mockPullChanges.mockRejectedValue(new Error('Pull failed')); + + const { result } = renderHook(() => useGitOperations()); + + let pullResult: boolean | undefined; + await act(async () => { + pullResult = await result.current.handlePull(); + }); + + expect(pullResult).toBe(false); + expect(consoleSpy).toHaveBeenCalledWith( + 'Failed to pull latest changes:', + expect.any(Error) + ); + expect(notifications.show).toHaveBeenCalledWith({ + title: 'Error', + message: 'Failed to pull latest changes', + color: 'red', + }); + + consoleSpy.mockRestore(); + }); + + it('returns false when no workspace is available', async () => { + mockWorkspaceData.currentWorkspace = null; + + const { result } = renderHook(() => useGitOperations()); + + let pullResult: boolean | undefined; + await act(async () => { + pullResult = await result.current.handlePull(); + }); + + expect(pullResult).toBe(false); + expect(gitApi.pullChanges).not.toHaveBeenCalled(); + }); + + it('returns false when git is disabled', async () => { + mockWorkspaceData.settings.gitEnabled = false; + + const { result } = renderHook(() => useGitOperations()); + + let pullResult: boolean | undefined; + await act(async () => { + pullResult = await result.current.handlePull(); + }); + + expect(pullResult).toBe(false); + expect(gitApi.pullChanges).not.toHaveBeenCalled(); + }); + + it('handles pull with different response messages', async () => { + const mockPullChanges = vi.mocked(gitApi.pullChanges); + mockPullChanges.mockResolvedValue('Already up to date'); + + const { result } = renderHook(() => useGitOperations()); + + await act(async () => { + await result.current.handlePull(); + }); + + expect(notifications.show).toHaveBeenCalledWith({ + title: 'Success', + message: 'Already up to date', + color: 'green', + }); + }); + }); + + describe('handleCommitAndPush', () => { + it('commits and pushes successfully with commit hash', async () => { + const mockCommitAndPush = vi.mocked(gitApi.commitAndPush); + mockCommitAndPush.mockResolvedValue('abc123def456'); + + const { result } = renderHook(() => useGitOperations()); + + await act(async () => { + await result.current.handleCommitAndPush('Add new feature'); + }); + + expect(mockCommitAndPush).toHaveBeenCalledWith( + 'test-workspace', + 'Add new feature' + ); + expect(notifications.show).toHaveBeenCalledWith({ + title: 'Success', + message: 'Successfully committed and pushed changes abc123def456', + color: 'green', + }); + }); + + it('handles commit errors and shows error notification', async () => { + const mockCommitAndPush = vi.mocked(gitApi.commitAndPush); + const consoleSpy = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); + + mockCommitAndPush.mockRejectedValue(new Error('Commit failed')); + + const { result } = renderHook(() => useGitOperations()); + + await act(async () => { + await result.current.handleCommitAndPush('Failed commit'); + }); + + expect(consoleSpy).toHaveBeenCalledWith( + 'Failed to commit and push changes:', + expect.any(Error) + ); + expect(notifications.show).toHaveBeenCalledWith({ + title: 'Error', + message: 'Failed to commit and push changes', + color: 'red', + }); + + consoleSpy.mockRestore(); + }); + + it('does nothing when no workspace is available', async () => { + mockWorkspaceData.currentWorkspace = null; + + const { result } = renderHook(() => useGitOperations()); + + await act(async () => { + await result.current.handleCommitAndPush('Test commit'); + }); + + expect(gitApi.commitAndPush).not.toHaveBeenCalled(); + expect(notifications.show).not.toHaveBeenCalled(); + }); + + it('does nothing when git is disabled', async () => { + mockWorkspaceData.settings.gitEnabled = false; + + const { result } = renderHook(() => useGitOperations()); + + await act(async () => { + await result.current.handleCommitAndPush('Test commit'); + }); + + expect(gitApi.commitAndPush).not.toHaveBeenCalled(); + expect(notifications.show).not.toHaveBeenCalled(); + }); + + it('handles empty commit messages', async () => { + const mockCommitAndPush = vi.mocked(gitApi.commitAndPush); + mockCommitAndPush.mockResolvedValue('xyz789abc123'); + + const { result } = renderHook(() => useGitOperations()); + + await act(async () => { + await result.current.handleCommitAndPush(''); + }); + + expect(mockCommitAndPush).toHaveBeenCalledWith('test-workspace', ''); + expect(notifications.show).toHaveBeenCalledWith({ + title: 'Success', + message: 'Successfully committed and pushed changes xyz789abc123', + color: 'green', + }); + }); + + it('handles long commit messages', async () => { + const mockCommitAndPush = vi.mocked(gitApi.commitAndPush); + mockCommitAndPush.mockResolvedValue('longcommithash123456789'); + + const longMessage = + 'This is a very long commit message that describes in detail all the changes that were made to the codebase including bug fixes, new features, and documentation updates'; + + const { result } = renderHook(() => useGitOperations()); + + await act(async () => { + await result.current.handleCommitAndPush(longMessage); + }); + + expect(mockCommitAndPush).toHaveBeenCalledWith( + 'test-workspace', + longMessage + ); + expect(notifications.show).toHaveBeenCalledWith({ + title: 'Success', + message: + 'Successfully committed and pushed changes longcommithash123456789', + color: 'green', + }); + }); + + it('handles commit with special characters in message', async () => { + const mockCommitAndPush = vi.mocked(gitApi.commitAndPush); + mockCommitAndPush.mockResolvedValue('special123hash'); + + const specialMessage = + 'Fix: update file with special chars àáâãäå & symbols!@#$%'; + + const { result } = renderHook(() => useGitOperations()); + + await act(async () => { + await result.current.handleCommitAndPush(specialMessage); + }); + + expect(mockCommitAndPush).toHaveBeenCalledWith( + 'test-workspace', + specialMessage + ); + expect(notifications.show).toHaveBeenCalledWith({ + title: 'Success', + message: 'Successfully committed and pushed changes special123hash', + color: 'green', + }); + }); + }); + + describe('workspace and settings dependencies', () => { + it('handles workspace changes correctly', async () => { + const mockPullChanges = vi.mocked(gitApi.pullChanges); + mockPullChanges.mockResolvedValue('Success'); + + const { result, rerender } = renderHook(() => useGitOperations()); + + // Test with initial workspace + await act(async () => { + await result.current.handlePull(); + }); + + expect(mockPullChanges).toHaveBeenCalledWith('test-workspace'); + + // Change workspace + mockWorkspaceData.currentWorkspace = { + id: 2, + name: 'different-workspace', + }; + + rerender(); + + await act(async () => { + await result.current.handlePull(); + }); + + expect(mockPullChanges).toHaveBeenCalledWith('different-workspace'); + }); + + it('handles git settings changes correctly', async () => { + const { result, rerender } = renderHook(() => useGitOperations()); + + // Initially git is enabled + expect(mockWorkspaceData.settings.gitEnabled).toBe(true); + + // Disable git + mockWorkspaceData.settings.gitEnabled = false; + rerender(); + + let pullResult: boolean | undefined; + await act(async () => { + pullResult = await result.current.handlePull(); + }); + + expect(pullResult).toBe(false); + expect(gitApi.pullChanges).not.toHaveBeenCalled(); + }); + }); + + describe('hook interface', () => { + it('returns correct function interface', () => { + const { result } = renderHook(() => useGitOperations()); + + expect(typeof result.current.handlePull).toBe('function'); + expect(typeof result.current.handleCommitAndPush).toBe('function'); + }); + + it('functions are stable across re-renders', () => { + const { result, rerender } = renderHook(() => useGitOperations()); + + const initialHandlers = { + handlePull: result.current.handlePull, + handleCommitAndPush: result.current.handleCommitAndPush, + }; + + rerender(); + + expect(result.current.handlePull).toBe(initialHandlers.handlePull); + expect(result.current.handleCommitAndPush).toBe( + initialHandlers.handleCommitAndPush + ); + }); + }); + + describe('edge cases', () => { + it('handles null workspace gracefully', async () => { + mockWorkspaceData.currentWorkspace = null; + + const { result } = renderHook(() => useGitOperations()); + + let pullResult: boolean | undefined; + await act(async () => { + pullResult = await result.current.handlePull(); + }); + + expect(pullResult).toBe(false); + expect(gitApi.pullChanges).not.toHaveBeenCalled(); + }); + + it('handles undefined workspace name gracefully', async () => { + mockWorkspaceData.currentWorkspace = { + id: 1, + name: undefined as unknown as string, + }; + + const { result } = renderHook(() => useGitOperations()); + + let pullResult: boolean | undefined; + await act(async () => { + pullResult = await result.current.handlePull(); + }); + + expect(pullResult).toBe(false); + expect(gitApi.pullChanges).not.toHaveBeenCalled(); + }); + + it('handles missing settings gracefully', async () => { + mockWorkspaceData.settings = { + gitEnabled: undefined as unknown as boolean, + }; + + const { result } = renderHook(() => useGitOperations()); + + let pullResult: boolean | undefined; + await act(async () => { + pullResult = await result.current.handlePull(); + }); + + expect(pullResult).toBe(false); + expect(gitApi.pullChanges).not.toHaveBeenCalled(); + }); + + it('handles API returning non-string commit hash', async () => { + const mockCommitAndPush = vi.mocked(gitApi.commitAndPush); + // API might return something unexpected + mockCommitAndPush.mockResolvedValue(null as unknown as string); + + const { result } = renderHook(() => useGitOperations()); + + await act(async () => { + await result.current.handleCommitAndPush('Test commit'); + }); + + expect(notifications.show).toHaveBeenCalledWith({ + title: 'Success', + message: 'Successfully committed and pushed changes null', + color: 'green', + }); + }); + }); +}); diff --git a/app/src/hooks/useGitOperations.ts b/app/src/hooks/useGitOperations.ts index 4e6d06f..0745606 100644 --- a/app/src/hooks/useGitOperations.ts +++ b/app/src/hooks/useGitOperations.ts @@ -13,7 +13,8 @@ export const useGitOperations = (): UseGitOperationsResult => { const { currentWorkspace, settings } = useWorkspaceData(); const handlePull = useCallback(async (): Promise => { - if (!currentWorkspace || !settings.gitEnabled) return false; + if (!currentWorkspace || !settings.gitEnabled || !currentWorkspace.name) + return false; try { const message = await pullChanges(currentWorkspace.name); @@ -37,11 +38,12 @@ export const useGitOperations = (): UseGitOperationsResult => { const handleCommitAndPush = useCallback( async (message: string): Promise => { if (!currentWorkspace || !settings.gitEnabled) return; - const commitHash: CommitHash = await commitAndPush( - currentWorkspace.name, - message - ); + try { + const commitHash: CommitHash = await commitAndPush( + currentWorkspace.name, + message + ); notifications.show({ title: 'Success', message: 'Successfully committed and pushed changes ' + commitHash, diff --git a/app/src/utils/fileHelpers.test.ts b/app/src/utils/fileHelpers.test.ts index f370d64..f1fc0ab 100644 --- a/app/src/utils/fileHelpers.test.ts +++ b/app/src/utils/fileHelpers.test.ts @@ -157,8 +157,6 @@ describe('fileHelpers', () => { }); it('uses the API base URL correctly', () => { - // Test that the function uses the expected API base URL - // Note: The API_BASE_URL is imported at module load time, so we test the expected behavior const url = getFileUrl('test', 'file.md'); expect(url).toBe( 'http://localhost:8080/api/v1/workspaces/test/files/file.md' diff --git a/app/src/utils/remarkWikiLinks.test.ts b/app/src/utils/remarkWikiLinks.test.ts index cf72816..113c645 100644 --- a/app/src/utils/remarkWikiLinks.test.ts +++ b/app/src/utils/remarkWikiLinks.test.ts @@ -278,6 +278,7 @@ describe('remarkWikiLinks', () => { expect(result.toString()).toContain('Spaces'); // Should not call API for empty/whitespace-only links + expect(fileApi.lookupFileByName).not.toHaveBeenCalled(); }); it('handles nested brackets', async () => { @@ -319,19 +320,5 @@ describe('remarkWikiLinks', () => { // Should not call API when workspace is empty expect(fileApi.lookupFileByName).not.toHaveBeenCalled(); }); - - it('does not process links when workspace is not provided', async () => { - const processor = unified() - .use(remarkParse) - .use(remarkWikiLinks, '') - .use(remarkStringify); - - const markdown = '[[test]]'; - - const result = await processor.process(markdown); - - expect(result.toString()).toContain('test'); - expect(fileApi.lookupFileByName).not.toHaveBeenCalled(); - }); }); }); diff --git a/app/src/utils/remarkWikiLinks.ts b/app/src/utils/remarkWikiLinks.ts index 43b3a1d..8a5fbe8 100644 --- a/app/src/utils/remarkWikiLinks.ts +++ b/app/src/utils/remarkWikiLinks.ts @@ -233,6 +233,13 @@ export function remarkWikiLinks(workspaceName: string) { } try { + // Skip API call for empty or whitespace-only filenames + if (!match.fileName.trim()) { + newNodes.push(createTextNode(match.fullMatch)); + lastIndex = match.index + match.fullMatch.length; + continue; + } + const lookupFileName: string = match.isImage ? match.fileName : addMarkdownExtension(match.fileName);