From 6251de8e19b7c03908bf2b9ba4d0bf1064e9fac4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C4=86ori=C4=87?= Date: Fri, 13 Feb 2026 16:42:34 +0100 Subject: [PATCH 1/7] feat: enhance error handling in git-push and repo services; add tests for error scenarios --- src/ui/services/git-push.ts | 91 +++++++++--------- src/ui/services/repo.ts | 52 ++++++++--- src/ui/views/PushDetails/PushDetails.tsx | 36 +++++--- src/ui/views/RepoDetails/RepoDetails.tsx | 10 +- test/ui/git-push.test.ts | 113 +++++++++++++++++++++++ test/ui/repo.test.ts | 108 ++++++++++++++++++++++ 6 files changed, 335 insertions(+), 75 deletions(-) create mode 100644 test/ui/git-push.test.ts create mode 100644 test/ui/repo.test.ts diff --git a/src/ui/services/git-push.ts b/src/ui/services/git-push.ts index eafe5c96d..7d75ccfb6 100644 --- a/src/ui/services/git-push.ts +++ b/src/ui/services/git-push.ts @@ -1,9 +1,29 @@ import axios from 'axios'; import { getAxiosConfig, processAuthError } from './auth'; -import { getBaseUrl, getApiV1BaseUrl } from './apiConfig'; +import { getApiV1BaseUrl } from './apiConfig'; import { Action, Step } from '../../proxy/actions'; import { PushActionView } from '../types'; +interface PushActionResult { + success: boolean; + status?: number; + message?: string; +} + +const getActionErrorResult = (error: any, fallbackMessage: string): PushActionResult => { + const status = error?.response?.status; + const responseMessage = error?.response?.data?.message; + const message = + typeof responseMessage === 'string' && responseMessage.trim().length > 0 + ? responseMessage + : error?.message || fallbackMessage; + return { + success: false, + status, + message, + }; +}; + const getPush = async ( id: string, setIsLoading: (isLoading: boolean) => void, @@ -67,16 +87,13 @@ const getPushes = async ( const authorisePush = async ( id: string, - setMessage: (message: string) => void, - setUserAllowedToApprove: (userAllowedToApprove: boolean) => void, attestation: Array<{ label: string; checked: boolean }>, -): Promise => { +): Promise => { const apiV1Base = await getApiV1BaseUrl(); const url = `${apiV1Base}/push/${id}/authorise`; - let errorMsg = ''; - let isUserAllowedToApprove = true; - await axios - .post( + + try { + await axios.post( url, { params: { @@ -84,50 +101,36 @@ const authorisePush = async ( }, }, getAxiosConfig(), - ) - .catch((error: any) => { - if (error.response && error.response.status === 401) { - errorMsg = 'You are not authorised to approve...'; - isUserAllowedToApprove = false; - } - }); - setMessage(errorMsg); - setUserAllowedToApprove(isUserAllowedToApprove); + ); + return { success: true }; + } catch (error: any) { + return getActionErrorResult(error, 'Failed to approve push request'); + } }; -const rejectPush = async ( - id: string, - setMessage: (message: string) => void, - setUserAllowedToReject: (userAllowedToReject: boolean) => void, -): Promise => { +const rejectPush = async (id: string): Promise => { const apiV1Base = await getApiV1BaseUrl(); const url = `${apiV1Base}/push/${id}/reject`; - let errorMsg = ''; - let isUserAllowedToReject = true; - await axios.post(url, {}, getAxiosConfig()).catch((error: any) => { - if (error.response && error.response.status === 401) { - errorMsg = 'You are not authorised to reject...'; - isUserAllowedToReject = false; - } - }); - setMessage(errorMsg); - setUserAllowedToReject(isUserAllowedToReject); + + try { + await axios.post(url, {}, getAxiosConfig()); + return { success: true }; + } catch (error: any) { + return getActionErrorResult(error, 'Failed to reject push request'); + } }; -const cancelPush = async ( - id: string, - setAuth: (auth: boolean) => void, - setIsError: (isError: boolean) => void, -): Promise => { +const cancelPush = async (id: string): Promise => { const apiV1Base = await getApiV1BaseUrl(); const url = `${apiV1Base}/push/${id}/cancel`; - await axios.post(url, {}, getAxiosConfig()).catch((error: any) => { - if (error.response && error.response.status === 401) { - setAuth(false); - } else { - setIsError(true); - } - }); + + try { + await axios.post(url, {}, getAxiosConfig()); + return { success: true }; + } catch (error: any) { + return getActionErrorResult(error, 'Failed to cancel push request'); + } }; export { getPush, getPushes, authorisePush, rejectPush, cancelPush }; +export type { PushActionResult }; diff --git a/src/ui/services/repo.ts b/src/ui/services/repo.ts index 8aa883d39..a52e974c4 100644 --- a/src/ui/services/repo.ts +++ b/src/ui/services/repo.ts @@ -4,6 +4,24 @@ import { Repo } from '../../db/types'; import { RepoView } from '../types'; import { getApiV1BaseUrl } from './apiConfig'; +interface ServiceError { + status?: number; + message: string; +} + +const getServiceError = (error: any, fallbackMessage: string): ServiceError => { + const status = error?.response?.status; + const responseMessage = error?.response?.data?.message; + const message = + typeof responseMessage === 'string' && responseMessage.trim().length > 0 + ? responseMessage + : error?.message || fallbackMessage; + return { status, message }; +}; + +const formatErrorMessage = (prefix: string, status: number | undefined, message: string): string => + `${prefix}: ${status ? `${status} ` : ''}${message}`; + const canAddUser = async (repoId: string, user: string, action: string) => { const apiV1Base = await getApiV1BaseUrl(); const url = new URL(`${apiV1Base}/repo/${repoId}`); @@ -18,7 +36,8 @@ const canAddUser = async (repoId: string, user: string, action: string) => { } }) .catch((error: any) => { - throw error; + const { message } = getServiceError(error, 'Failed to validate repo permissions'); + throw new Error(message); }); }; @@ -50,11 +69,12 @@ const getRepos = async ( }) .catch((error: any) => { setIsError(true); - if (error.response && error.response.status === 401) { + const { status, message } = getServiceError(error, 'Unknown error'); + if (status === 401) { setAuth(false); setErrorMessage(processAuthError(error)); } else { - setErrorMessage(`Error fetching repos: ${error.response.data.message}`); + setErrorMessage(formatErrorMessage('Error fetching repos', status, message)); } }) .finally(() => { @@ -67,6 +87,7 @@ const getRepo = async ( setRepo: (repo: RepoView) => void, setAuth: (auth: boolean) => void, setIsError: (isError: boolean) => void, + setErrorMessage: (errorMessage: string) => void, id: string, ): Promise => { const apiV1Base = await getApiV1BaseUrl(); @@ -78,10 +99,13 @@ const getRepo = async ( setRepo(repo); }) .catch((error: any) => { - if (error.response && error.response.status === 401) { + const { status, message } = getServiceError(error, 'Unknown error'); + setIsError(true); + if (status === 401) { setAuth(false); + setErrorMessage(processAuthError(error)); } else { - setIsError(true); + setErrorMessage(formatErrorMessage('Error fetching repo', status, message)); } }) .finally(() => { @@ -102,9 +126,10 @@ const addRepo = async ( repo: response.data, }; } catch (error: any) { + const { message } = getServiceError(error, 'Failed to add repository'); return { success: false, - message: error.response?.data?.message || error.message, + message, repo: null, }; } @@ -117,8 +142,9 @@ const addUser = async (repoId: string, user: string, action: string): Promise { - console.log(error.response.data.message); - throw error; + const { message } = getServiceError(error, 'Failed to add user'); + console.log(message); + throw new Error(message); }); } else { console.log('Duplicate user can not be added'); @@ -131,8 +157,9 @@ const deleteUser = async (user: string, repoId: string, action: string): Promise const url = new URL(`${apiV1Base}/repo/${repoId}/user/${action}/${user}`); await axios.delete(url.toString(), getAxiosConfig()).catch((error: any) => { - console.log(error.response.data.message); - throw error; + const { message } = getServiceError(error, 'Failed to remove user'); + console.log(message); + throw new Error(message); }); }; @@ -141,8 +168,9 @@ const deleteRepo = async (repoId: string): Promise => { const url = new URL(`${apiV1Base}/repo/${repoId}/delete`); await axios.delete(url.toString(), getAxiosConfig()).catch((error: any) => { - console.log(error.response.data.message); - throw error; + const { message } = getServiceError(error, 'Failed to delete repository'); + console.log(message); + throw new Error(message); }); }; diff --git a/src/ui/views/PushDetails/PushDetails.tsx b/src/ui/views/PushDetails/PushDetails.tsx index aec01fa20..fc09bb5d8 100644 --- a/src/ui/views/PushDetails/PushDetails.tsx +++ b/src/ui/views/PushDetails/PushDetails.tsx @@ -19,6 +19,7 @@ import TableHead from '@material-ui/core/TableHead'; import TableRow from '@material-ui/core/TableRow'; import TableCell from '@material-ui/core/TableCell'; import { getPush, authorisePush, rejectPush, cancelPush } from '../../services/git-push'; +import type { PushActionResult } from '../../services/git-push'; import { CheckCircle, Visibility, Cancel, Block } from '@material-ui/icons'; import Snackbar from '@material-ui/core/Snackbar'; import Tooltip from '@material-ui/core/Tooltip'; @@ -37,15 +38,12 @@ const Dashboard: React.FC = () => { const [attestation, setAttestation] = useState(false); const navigate = useNavigate(); - let isUserAllowedToApprove = true; - let isUserAllowedToReject = true; - - const setUserAllowedToApprove = (userAllowedToApprove: boolean) => { - isUserAllowedToApprove = userAllowedToApprove; - }; - - const setUserAllowedToReject = (userAllowedToReject: boolean) => { - isUserAllowedToReject = userAllowedToReject; + const handlePushActionFailure = (result: PushActionResult) => { + if (result.status === 401) { + navigate('/login', { replace: true }); + return; + } + setMessage(result.message || 'Something went wrong...'); }; useEffect(() => { @@ -56,24 +54,32 @@ const Dashboard: React.FC = () => { const authorise = async (attestationData: Array<{ label: string; checked: boolean }>) => { if (!id) return; - await authorisePush(id, setMessage, setUserAllowedToApprove, attestationData); - if (isUserAllowedToApprove) { + const result = await authorisePush(id, attestationData); + if (result.success) { navigate('/dashboard/push/'); + return; } + handlePushActionFailure(result); }; const reject = async () => { if (!id) return; - await rejectPush(id, setMessage, setUserAllowedToReject); - if (isUserAllowedToReject) { + const result = await rejectPush(id); + if (result.success) { navigate('/dashboard/push/'); + return; } + handlePushActionFailure(result); }; const cancel = async () => { if (!id) return; - await cancelPush(id, setAuth, setIsError); - navigate(`/dashboard/push/`); + const result = await cancelPush(id); + if (result.success) { + navigate(`/dashboard/push/`); + return; + } + handlePushActionFailure(result); }; if (isLoading) return
Loading...
; diff --git a/src/ui/views/RepoDetails/RepoDetails.tsx b/src/ui/views/RepoDetails/RepoDetails.tsx index c5c1c2ccb..5ed3788a3 100644 --- a/src/ui/views/RepoDetails/RepoDetails.tsx +++ b/src/ui/views/RepoDetails/RepoDetails.tsx @@ -27,6 +27,7 @@ import { RepoView, SCMRepositoryMetadata } from '../../types'; import { UserContextType } from '../../context'; import UserLink from '../../components/UserLink/UserLink'; import DeleteRepoDialog from './Components/DeleteRepoDialog'; +import Danger from '../../components/Typography/Danger'; const useStyles = makeStyles((theme) => ({ root: { @@ -48,13 +49,14 @@ const RepoDetails: React.FC = () => { const [, setAuth] = useState(true); const [isLoading, setIsLoading] = useState(true); const [isError, setIsError] = useState(false); + const [errorMessage, setErrorMessage] = useState(''); const [remoteRepoData, setRemoteRepoData] = useState(null); const { user } = useContext(UserContext); const { id: repoId } = useParams<{ id: string }>(); useEffect(() => { if (repoId) { - getRepo(setIsLoading, setRepo, setAuth, setIsError, repoId); + getRepo(setIsLoading, setRepo, setAuth, setIsError, setErrorMessage, repoId); } }, [repoId]); @@ -67,7 +69,7 @@ const RepoDetails: React.FC = () => { const removeUser = async (userToRemove: string, action: 'authorise' | 'push') => { if (!repoId) return; await deleteUser(userToRemove, repoId, action); - getRepo(setIsLoading, setRepo, setAuth, setIsError, repoId); + getRepo(setIsLoading, setRepo, setAuth, setIsError, setErrorMessage, repoId); }; const removeRepository = async (id: string) => { @@ -77,12 +79,12 @@ const RepoDetails: React.FC = () => { const refresh = () => { if (repoId) { - getRepo(setIsLoading, setRepo, setAuth, setIsError, repoId); + getRepo(setIsLoading, setRepo, setAuth, setIsError, setErrorMessage, repoId); } }; if (isLoading) return
Loading...
; - if (isError) return
Something went wrong ...
; + if (isError) return {errorMessage || 'Something went wrong ...'}; if (!repo) return
No repository data found
; const { url: remoteUrl, proxyURL } = repo || {}; diff --git a/test/ui/git-push.test.ts b/test/ui/git-push.test.ts new file mode 100644 index 000000000..c7eb59258 --- /dev/null +++ b/test/ui/git-push.test.ts @@ -0,0 +1,113 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import axios from 'axios'; +import { authorisePush, cancelPush, rejectPush } from '../../src/ui/services/git-push'; + +vi.mock('axios', () => ({ + default: { + post: vi.fn(), + }, +})); + +vi.mock('../../src/ui/services/apiConfig', () => ({ + getApiV1BaseUrl: vi.fn(async () => 'http://localhost:8080/api/v1'), +})); + +vi.mock('../../src/ui/services/auth', () => ({ + getAxiosConfig: vi.fn(() => ({ + withCredentials: true, + headers: {}, + })), + processAuthError: vi.fn(), +})); + +describe('git-push service action errors', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('returns success for authorise action', async () => { + vi.mocked(axios.post).mockResolvedValue({ data: {} } as any); + + const result = await authorisePush('push-123', [{ label: 'LGTM', checked: true }]); + + expect(result).toEqual({ success: true }); + expect(axios.post).toHaveBeenCalledWith( + 'http://localhost:8080/api/v1/push/push-123/authorise', + { + params: { + attestation: [{ label: 'LGTM', checked: true }], + }, + }, + expect.any(Object), + ); + }); + + it('returns backend not-logged-in message for authorise 401 errors', async () => { + vi.mocked(axios.post).mockRejectedValue({ + response: { + status: 401, + data: { + message: 'Not logged in', + }, + }, + }); + + const result = await authorisePush('push-123', []); + + expect(result).toEqual({ + success: false, + status: 401, + message: 'Not logged in', + }); + }); + + it('returns backend message for reject 403 errors', async () => { + vi.mocked(axios.post).mockRejectedValue({ + response: { + status: 403, + data: { + message: 'User alice is not authorised to reject changes on this project', + }, + }, + }); + + const result = await rejectPush('push-456'); + + expect(result).toEqual({ + success: false, + status: 403, + message: 'User alice is not authorised to reject changes on this project', + }); + }); + + it('returns backend message for cancel 401 errors', async () => { + vi.mocked(axios.post).mockRejectedValue({ + response: { + status: 401, + data: { + message: 'Not logged in', + }, + }, + }); + + const result = await cancelPush('push-789'); + + expect(result).toEqual({ + success: false, + status: 401, + message: 'Not logged in', + }); + }); + + it('falls back to thrown error message when response payload is missing', async () => { + vi.mocked(axios.post).mockRejectedValue(new Error('network timeout')); + + const result = await rejectPush('push-999'); + + expect(result).toEqual({ + success: false, + status: undefined, + message: 'network timeout', + }); + }); +}); diff --git a/test/ui/repo.test.ts b/test/ui/repo.test.ts new file mode 100644 index 000000000..4269a80cf --- /dev/null +++ b/test/ui/repo.test.ts @@ -0,0 +1,108 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { addUser, getRepo } from '../../src/ui/services/repo'; + +const { axiosMock, processAuthErrorMock } = vi.hoisted(() => { + const axiosFn = vi.fn() as any; + axiosFn.get = vi.fn(); + axiosFn.post = vi.fn(); + axiosFn.patch = vi.fn(); + axiosFn.delete = vi.fn(); + + return { + axiosMock: axiosFn, + processAuthErrorMock: vi.fn(() => 'Failed to authorize user: Not logged in.'), + }; +}); + +vi.mock('axios', () => ({ + default: axiosMock, +})); + +vi.mock('../../src/ui/services/auth.js', () => ({ + getAxiosConfig: vi.fn(() => ({ + withCredentials: true, + headers: {}, + })), + processAuthError: processAuthErrorMock, +})); + +vi.mock('../../src/ui/services/apiConfig', () => ({ + getApiV1BaseUrl: vi.fn(async () => 'http://localhost:8080/api/v1'), +})); + +describe('repo service error handling', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('sets detailed error message when getRepo fails with non-401 status', async () => { + axiosMock.mockRejectedValue({ + response: { + status: 403, + data: { + message: 'User alice not authorised on this repository', + }, + }, + }); + + const setIsLoading = vi.fn(); + const setRepo = vi.fn(); + const setAuth = vi.fn(); + const setIsError = vi.fn(); + const setErrorMessage = vi.fn(); + + await getRepo(setIsLoading, setRepo, setAuth, setIsError, setErrorMessage, 'repo-1'); + + expect(setIsError).toHaveBeenCalledWith(true); + expect(setErrorMessage).toHaveBeenCalledWith( + 'Error fetching repo: 403 User alice not authorised on this repository', + ); + expect(setAuth).not.toHaveBeenCalledWith(false); + }); + + it('uses processAuthError when getRepo fails with 401 status', async () => { + axiosMock.mockRejectedValue({ + response: { + status: 401, + data: { + message: 'Not logged in', + }, + }, + }); + + const setIsLoading = vi.fn(); + const setRepo = vi.fn(); + const setAuth = vi.fn(); + const setIsError = vi.fn(); + const setErrorMessage = vi.fn(); + + await getRepo(setIsLoading, setRepo, setAuth, setIsError, setErrorMessage, 'repo-1'); + + expect(setIsError).toHaveBeenCalledWith(true); + expect(setAuth).toHaveBeenCalledWith(false); + expect(processAuthErrorMock).toHaveBeenCalled(); + expect(setErrorMessage).toHaveBeenCalledWith('Failed to authorize user: Not logged in.'); + }); + + it('throws backend message when addUser patch request fails', async () => { + axiosMock.get.mockResolvedValue({ + data: { + users: { + canAuthorise: [], + canPush: [], + }, + }, + }); + + axiosMock.patch.mockRejectedValue({ + response: { + status: 404, + data: { + message: 'User bob not found', + }, + }, + }); + + await expect(addUser('repo-1', 'bob', 'authorise')).rejects.toThrow('User bob not found'); + }); +}); From cfc3ff800a3e7c4db8f2d4c3ac87a2ed9b0998ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C4=86ori=C4=87?= Date: Fri, 13 Feb 2026 18:13:50 +0100 Subject: [PATCH 2/7] test: add comprehensive test coverage for UI service layer - Add new test file for errors.ts utility functions (18 tests) - Expand git-push.test.ts to cover getPush and getPushes functions (13 tests total) - Expand repo.test.ts to cover getRepos, addRepo, deleteUser, and deleteRepo (21 tests total) - Add new test file for user.ts service functions (13 tests) - All tests verify both success and error handling scenarios - Total test count increased from 14 to 79 tests for UI services --- src/ui/services/errors.ts | 35 ++ src/ui/services/git-push.ts | 81 +---- src/ui/services/repo.ts | 110 ++---- src/ui/services/user.ts | 23 +- src/ui/views/PushDetails/PushDetails.tsx | 33 +- .../PushRequests/components/PushesTable.tsx | 17 +- src/ui/views/RepoDetails/RepoDetails.tsx | 33 +- src/ui/views/RepoList/Components/NewRepo.tsx | 4 +- .../RepoList/Components/Repositories.tsx | 27 +- test/ui/errors.test.ts | 223 ++++++++++++ test/ui/git-push.test.ts | 284 +++++++++++---- test/ui/repo.test.ts | 328 ++++++++++++++++-- test/ui/user.test.ts | 265 ++++++++++++++ 13 files changed, 1175 insertions(+), 288 deletions(-) create mode 100644 src/ui/services/errors.ts create mode 100644 test/ui/errors.test.ts create mode 100644 test/ui/user.test.ts diff --git a/src/ui/services/errors.ts b/src/ui/services/errors.ts new file mode 100644 index 000000000..12554240e --- /dev/null +++ b/src/ui/services/errors.ts @@ -0,0 +1,35 @@ +export interface ServiceResult { + success: boolean; + status?: number; + message?: string; + data?: T; +} + +export const getServiceError = ( + error: any, + fallbackMessage: string, +): { status?: number; message: string } => { + const status = error?.response?.status; + const responseMessage = error?.response?.data?.message; + const message = + typeof responseMessage === 'string' && responseMessage.trim().length > 0 + ? responseMessage + : error?.message || fallbackMessage; + return { status, message }; +}; + +export const formatErrorMessage = ( + prefix: string, + status: number | undefined, + message: string, +): string => `${prefix}: ${status ? `${status} ` : ''}${message}`; + +export const errorResult = (error: any, fallbackMessage: string): ServiceResult => { + const { status, message } = getServiceError(error, fallbackMessage); + return { success: false, status, message }; +}; + +export const successResult = (data?: T): ServiceResult => ({ + success: true, + ...(data !== undefined && { data }), +}); diff --git a/src/ui/services/git-push.ts b/src/ui/services/git-push.ts index 7d75ccfb6..ccae6d7ef 100644 --- a/src/ui/services/git-push.ts +++ b/src/ui/services/git-push.ts @@ -1,94 +1,48 @@ import axios from 'axios'; -import { getAxiosConfig, processAuthError } from './auth'; +import { getAxiosConfig } from './auth'; import { getApiV1BaseUrl } from './apiConfig'; import { Action, Step } from '../../proxy/actions'; import { PushActionView } from '../types'; +import { ServiceResult, errorResult, successResult } from './errors'; -interface PushActionResult { - success: boolean; - status?: number; - message?: string; -} - -const getActionErrorResult = (error: any, fallbackMessage: string): PushActionResult => { - const status = error?.response?.status; - const responseMessage = error?.response?.data?.message; - const message = - typeof responseMessage === 'string' && responseMessage.trim().length > 0 - ? responseMessage - : error?.message || fallbackMessage; - return { - success: false, - status, - message, - }; -}; - -const getPush = async ( - id: string, - setIsLoading: (isLoading: boolean) => void, - setPush: (push: PushActionView) => void, - setAuth: (auth: boolean) => void, - setIsError: (isError: boolean) => void, -): Promise => { +const getPush = async (id: string): Promise> => { const apiV1Base = await getApiV1BaseUrl(); const url = `${apiV1Base}/push/${id}`; - setIsLoading(true); try { const response = await axios(url, getAxiosConfig()); const data: Action & { diff?: Step } = response.data; data.diff = data.steps.find((x: Step) => x.stepName === 'diff'); - setPush(data as PushActionView); + return successResult(data as PushActionView); } catch (error: any) { - if (error.response?.status === 401) setAuth(false); - else setIsError(true); - } finally { - setIsLoading(false); + return errorResult(error, 'Failed to load push'); } }; const getPushes = async ( - setIsLoading: (isLoading: boolean) => void, - setPushes: (pushes: PushActionView[]) => void, - setAuth: (auth: boolean) => void, - setIsError: (isError: boolean) => void, - setErrorMessage: (errorMessage: string) => void, query = { blocked: true, canceled: false, authorised: false, rejected: false, }, -): Promise => { +): Promise> => { const apiV1Base = await getApiV1BaseUrl(); const url = new URL(`${apiV1Base}/push`); url.search = new URLSearchParams(query as any).toString(); - setIsLoading(true); - try { const response = await axios(url.toString(), getAxiosConfig()); - setPushes(response.data as PushActionView[]); + return successResult(response.data as PushActionView[]); } catch (error: any) { - setIsError(true); - - if (error.response?.status === 401) { - setAuth(false); - setErrorMessage(processAuthError(error)); - } else { - const message = error.response?.data?.message || error.message; - setErrorMessage(`Error fetching pushes: ${message}`); - } - } finally { - setIsLoading(false); + return errorResult(error, 'Failed to load pushes'); } }; const authorisePush = async ( id: string, attestation: Array<{ label: string; checked: boolean }>, -): Promise => { +): Promise => { const apiV1Base = await getApiV1BaseUrl(); const url = `${apiV1Base}/push/${id}/authorise`; @@ -102,35 +56,34 @@ const authorisePush = async ( }, getAxiosConfig(), ); - return { success: true }; + return successResult(); } catch (error: any) { - return getActionErrorResult(error, 'Failed to approve push request'); + return errorResult(error, 'Failed to approve push request'); } }; -const rejectPush = async (id: string): Promise => { +const rejectPush = async (id: string): Promise => { const apiV1Base = await getApiV1BaseUrl(); const url = `${apiV1Base}/push/${id}/reject`; try { await axios.post(url, {}, getAxiosConfig()); - return { success: true }; + return successResult(); } catch (error: any) { - return getActionErrorResult(error, 'Failed to reject push request'); + return errorResult(error, 'Failed to reject push request'); } }; -const cancelPush = async (id: string): Promise => { +const cancelPush = async (id: string): Promise => { const apiV1Base = await getApiV1BaseUrl(); const url = `${apiV1Base}/push/${id}/cancel`; try { await axios.post(url, {}, getAxiosConfig()); - return { success: true }; + return successResult(); } catch (error: any) { - return getActionErrorResult(error, 'Failed to cancel push request'); + return errorResult(error, 'Failed to cancel push request'); } }; export { getPush, getPushes, authorisePush, rejectPush, cancelPush }; -export type { PushActionResult }; diff --git a/src/ui/services/repo.ts b/src/ui/services/repo.ts index a52e974c4..9b5a36323 100644 --- a/src/ui/services/repo.ts +++ b/src/ui/services/repo.ts @@ -1,26 +1,9 @@ import axios from 'axios'; -import { getAxiosConfig, processAuthError } from './auth.js'; +import { getAxiosConfig } from './auth.js'; import { Repo } from '../../db/types'; import { RepoView } from '../types'; import { getApiV1BaseUrl } from './apiConfig'; - -interface ServiceError { - status?: number; - message: string; -} - -const getServiceError = (error: any, fallbackMessage: string): ServiceError => { - const status = error?.response?.status; - const responseMessage = error?.response?.data?.message; - const message = - typeof responseMessage === 'string' && responseMessage.trim().length > 0 - ? responseMessage - : error?.message || fallbackMessage; - return { status, message }; -}; - -const formatErrorMessage = (prefix: string, status: number | undefined, message: string): string => - `${prefix}: ${status ? `${status} ` : ''}${message}`; +import { ServiceResult, getServiceError, errorResult, successResult } from './errors'; const canAddUser = async (repoId: string, user: string, action: string) => { const apiV1Base = await getApiV1BaseUrl(); @@ -49,89 +32,44 @@ class DupUserValidationError extends Error { } const getRepos = async ( - setIsLoading: (isLoading: boolean) => void, - setRepos: (repos: RepoView[]) => void, - setAuth: (auth: boolean) => void, - setIsError: (isError: boolean) => void, - setErrorMessage: (errorMessage: string) => void, query: Record = {}, -): Promise => { +): Promise> => { const apiV1Base = await getApiV1BaseUrl(); const url = new URL(`${apiV1Base}/repo`); url.search = new URLSearchParams(query as any).toString(); - setIsLoading(true); - await axios(url.toString(), getAxiosConfig()) - .then((response) => { - const sortedRepos = response.data.sort((a: RepoView, b: RepoView) => - a.name.localeCompare(b.name), - ); - setRepos(sortedRepos); - }) - .catch((error: any) => { - setIsError(true); - const { status, message } = getServiceError(error, 'Unknown error'); - if (status === 401) { - setAuth(false); - setErrorMessage(processAuthError(error)); - } else { - setErrorMessage(formatErrorMessage('Error fetching repos', status, message)); - } - }) - .finally(() => { - setIsLoading(false); - }); + + try { + const response = await axios(url.toString(), getAxiosConfig()); + const sortedRepos = response.data.sort((a: RepoView, b: RepoView) => + a.name.localeCompare(b.name), + ); + return successResult(sortedRepos); + } catch (error: any) { + return errorResult(error, 'Failed to load repositories'); + } }; -const getRepo = async ( - setIsLoading: (isLoading: boolean) => void, - setRepo: (repo: RepoView) => void, - setAuth: (auth: boolean) => void, - setIsError: (isError: boolean) => void, - setErrorMessage: (errorMessage: string) => void, - id: string, -): Promise => { +const getRepo = async (id: string): Promise> => { const apiV1Base = await getApiV1BaseUrl(); const url = new URL(`${apiV1Base}/repo/${id}`); - setIsLoading(true); - await axios(url.toString(), getAxiosConfig()) - .then((response) => { - const repo = response.data; - setRepo(repo); - }) - .catch((error: any) => { - const { status, message } = getServiceError(error, 'Unknown error'); - setIsError(true); - if (status === 401) { - setAuth(false); - setErrorMessage(processAuthError(error)); - } else { - setErrorMessage(formatErrorMessage('Error fetching repo', status, message)); - } - }) - .finally(() => { - setIsLoading(false); - }); + + try { + const response = await axios(url.toString(), getAxiosConfig()); + return successResult(response.data); + } catch (error: any) { + return errorResult(error, 'Failed to load repository'); + } }; -const addRepo = async ( - repo: RepoView, -): Promise<{ success: boolean; message?: string; repo: RepoView | null }> => { +const addRepo = async (repo: RepoView): Promise> => { const apiV1Base = await getApiV1BaseUrl(); const url = new URL(`${apiV1Base}/repo`); try { const response = await axios.post(url.toString(), repo, getAxiosConfig()); - return { - success: true, - repo: response.data, - }; + return successResult(response.data); } catch (error: any) { - const { message } = getServiceError(error, 'Failed to add repository'); - return { - success: false, - message, - repo: null, - }; + return errorResult(error, 'Failed to add repository'); } }; diff --git a/src/ui/services/user.ts b/src/ui/services/user.ts index 40c0394b5..39b066f2c 100644 --- a/src/ui/services/user.ts +++ b/src/ui/services/user.ts @@ -2,6 +2,7 @@ import axios, { AxiosError, AxiosResponse } from 'axios'; import { getAxiosConfig, processAuthError } from './auth'; import { PublicUser } from '../../db/types'; import { getBaseUrl, getApiV1BaseUrl } from './apiConfig'; +import { getServiceError, formatErrorMessage } from './errors'; type SetStateCallback = (value: T | ((prevValue: T) => T)) => void; @@ -27,14 +28,12 @@ const getUser = async ( setUser?.(user); setIsLoading?.(false); } catch (error) { - const axiosError = error as AxiosError; - const status = axiosError.response?.status; + const { status, message } = getServiceError(error, 'Unknown error'); if (status === 401) { setAuth?.(false); - setErrorMessage?.(processAuthError(axiosError)); + setErrorMessage?.(processAuthError(error as AxiosError)); } else { - const msg = (axiosError.response?.data as any)?.message ?? 'Unknown error'; - setErrorMessage?.(`Error fetching user: ${status} ${msg}`); + setErrorMessage?.(formatErrorMessage('Error fetching user', status, message)); } setIsLoading?.(false); } @@ -56,14 +55,12 @@ const getUsers = async ( ); setUsers(response.data); } catch (error) { - const axiosError = error as AxiosError; - const status = axiosError.response?.status; + const { status, message } = getServiceError(error, 'Unknown error'); if (status === 401) { setAuth(false); - setErrorMessage(processAuthError(axiosError)); + setErrorMessage(processAuthError(error as AxiosError)); } else { - const msg = (axiosError.response?.data as any)?.message ?? 'Unknown error'; - setErrorMessage(`Error fetching users: ${status} ${msg}`); + setErrorMessage(formatErrorMessage('Error fetching users', status, message)); } } finally { setIsLoading(false); @@ -79,10 +76,8 @@ const updateUser = async ( const baseUrl = await getBaseUrl(); await axios.post(`${baseUrl}/api/auth/gitAccount`, user, getAxiosConfig()); } catch (error) { - const axiosError = error as AxiosError; - const status = axiosError.response?.status; - const msg = (axiosError.response?.data as any)?.message ?? 'Unknown error'; - setErrorMessage(`Error updating user: ${status} ${msg}`); + const { status, message } = getServiceError(error, 'Unknown error'); + setErrorMessage(formatErrorMessage('Error updating user', status, message)); setIsLoading(false); } }; diff --git a/src/ui/views/PushDetails/PushDetails.tsx b/src/ui/views/PushDetails/PushDetails.tsx index fc09bb5d8..9320adc76 100644 --- a/src/ui/views/PushDetails/PushDetails.tsx +++ b/src/ui/views/PushDetails/PushDetails.tsx @@ -19,7 +19,7 @@ import TableHead from '@material-ui/core/TableHead'; import TableRow from '@material-ui/core/TableRow'; import TableCell from '@material-ui/core/TableCell'; import { getPush, authorisePush, rejectPush, cancelPush } from '../../services/git-push'; -import type { PushActionResult } from '../../services/git-push'; +import type { ServiceResult } from '../../services/errors'; import { CheckCircle, Visibility, Cancel, Block } from '@material-ui/icons'; import Snackbar from '@material-ui/core/Snackbar'; import Tooltip from '@material-ui/core/Tooltip'; @@ -27,18 +27,18 @@ import { AttestationFormData, PushActionView } from '../../types'; import { trimPrefixRefsHeads, trimTrailingDotGit } from '../../../db/helper'; import { generateEmailLink, getGitProvider } from '../../utils'; import UserLink from '../../components/UserLink/UserLink'; +import Danger from '../../components/Typography/Danger'; const Dashboard: React.FC = () => { const { id } = useParams<{ id: string }>(); const [push, setPush] = useState(null); - const [, setAuth] = useState(true); const [isLoading, setIsLoading] = useState(true); const [isError, setIsError] = useState(false); const [message, setMessage] = useState(''); const [attestation, setAttestation] = useState(false); const navigate = useNavigate(); - const handlePushActionFailure = (result: PushActionResult) => { + const handleActionFailure = (result: ServiceResult) => { if (result.status === 401) { navigate('/login', { replace: true }); return; @@ -47,9 +47,22 @@ const Dashboard: React.FC = () => { }; useEffect(() => { - if (id) { - getPush(id, setIsLoading, setPush, setAuth, setIsError); - } + if (!id) return; + const load = async () => { + setIsLoading(true); + const result = await getPush(id); + if (result.success && result.data) { + setPush(result.data); + } else if (result.status === 401) { + navigate('/login', { replace: true }); + return; + } else { + setIsError(true); + setMessage(result.message || 'Something went wrong...'); + } + setIsLoading(false); + }; + load(); }, [id]); const authorise = async (attestationData: Array<{ label: string; checked: boolean }>) => { @@ -59,7 +72,7 @@ const Dashboard: React.FC = () => { navigate('/dashboard/push/'); return; } - handlePushActionFailure(result); + handleActionFailure(result); }; const reject = async () => { @@ -69,7 +82,7 @@ const Dashboard: React.FC = () => { navigate('/dashboard/push/'); return; } - handlePushActionFailure(result); + handleActionFailure(result); }; const cancel = async () => { @@ -79,11 +92,11 @@ const Dashboard: React.FC = () => { navigate(`/dashboard/push/`); return; } - handlePushActionFailure(result); + handleActionFailure(result); }; if (isLoading) return
Loading...
; - if (isError) return
Something went wrong ...
; + if (isError) return {message || 'Something went wrong ...'}; if (!push) return
No push data found
; let headerData: { title: string; color: CardHeaderColor } = { diff --git a/src/ui/views/PushRequests/components/PushesTable.tsx b/src/ui/views/PushRequests/components/PushesTable.tsx index 88052c300..8f029de79 100644 --- a/src/ui/views/PushRequests/components/PushesTable.tsx +++ b/src/ui/views/PushRequests/components/PushesTable.tsx @@ -30,9 +30,7 @@ const PushesTable: React.FC = (props) => { const [pushes, setPushes] = useState([]); const [filteredData, setFilteredData] = useState([]); const [isLoading, setIsLoading] = useState(false); - const [, setIsError] = useState(false); const navigate = useNavigate(); - const [, setAuth] = useState(true); const [currentPage, setCurrentPage] = useState(1); const itemsPerPage = 5; const [searchTerm, setSearchTerm] = useState(''); @@ -49,7 +47,20 @@ const PushesTable: React.FC = (props) => { if (props.rejected !== undefined) query.rejected = props.rejected; if (props.error !== undefined) query.error = props.error; - getPushes(setIsLoading, setPushes, setAuth, setIsError, props.handleError, query); + const load = async () => { + setIsLoading(true); + const result = await getPushes(query); + if (result.success && result.data) { + setPushes(result.data); + } else if (result.status === 401) { + navigate('/login', { replace: true }); + return; + } else if (props.handleError) { + props.handleError(result.message || 'Failed to load pushes'); + } + setIsLoading(false); + }; + load(); }, [props]); useEffect(() => { diff --git a/src/ui/views/RepoDetails/RepoDetails.tsx b/src/ui/views/RepoDetails/RepoDetails.tsx index 5ed3788a3..4a6822a50 100644 --- a/src/ui/views/RepoDetails/RepoDetails.tsx +++ b/src/ui/views/RepoDetails/RepoDetails.tsx @@ -46,7 +46,6 @@ const RepoDetails: React.FC = () => { const classes = useStyles(); const [repo, setRepo] = useState(null); const [confirmDeleteOpen, setConfirmDeleteOpen] = useState(false); - const [, setAuth] = useState(true); const [isLoading, setIsLoading] = useState(true); const [isError, setIsError] = useState(false); const [errorMessage, setErrorMessage] = useState(''); @@ -55,9 +54,22 @@ const RepoDetails: React.FC = () => { const { id: repoId } = useParams<{ id: string }>(); useEffect(() => { - if (repoId) { - getRepo(setIsLoading, setRepo, setAuth, setIsError, setErrorMessage, repoId); - } + if (!repoId) return; + const load = async () => { + setIsLoading(true); + const result = await getRepo(repoId); + if (result.success && result.data) { + setRepo(result.data); + } else if (result.status === 401) { + navigate('/login', { replace: true }); + return; + } else { + setIsError(true); + setErrorMessage(result.message || 'Something went wrong...'); + } + setIsLoading(false); + }; + load(); }, [repoId]); useEffect(() => { @@ -69,7 +81,10 @@ const RepoDetails: React.FC = () => { const removeUser = async (userToRemove: string, action: 'authorise' | 'push') => { if (!repoId) return; await deleteUser(userToRemove, repoId, action); - getRepo(setIsLoading, setRepo, setAuth, setIsError, setErrorMessage, repoId); + const result = await getRepo(repoId); + if (result.success && result.data) { + setRepo(result.data); + } }; const removeRepository = async (id: string) => { @@ -77,9 +92,11 @@ const RepoDetails: React.FC = () => { navigate('/dashboard/repo', { replace: true }); }; - const refresh = () => { - if (repoId) { - getRepo(setIsLoading, setRepo, setAuth, setIsError, setErrorMessage, repoId); + const refresh = async () => { + if (!repoId) return; + const result = await getRepo(repoId); + if (result.success && result.data) { + setRepo(result.data); } }; diff --git a/src/ui/views/RepoList/Components/NewRepo.tsx b/src/ui/views/RepoList/Components/NewRepo.tsx index e29f8244f..f5f8ef4dc 100644 --- a/src/ui/views/RepoList/Components/NewRepo.tsx +++ b/src/ui/views/RepoList/Components/NewRepo.tsx @@ -85,8 +85,8 @@ const AddRepositoryDialog: React.FC = ({ open, onClose } const result = await addRepo(repo); - if (result.success && result.repo) { - handleSuccess(result.repo); + if (result.success && result.data) { + handleSuccess(result.data); handleClose(); } else { setError(result.message || 'Failed to add repository'); diff --git a/src/ui/views/RepoList/Components/Repositories.tsx b/src/ui/views/RepoList/Components/Repositories.tsx index a72cd2fc5..e4a4e89b7 100644 --- a/src/ui/views/RepoList/Components/Repositories.tsx +++ b/src/ui/views/RepoList/Components/Repositories.tsx @@ -37,7 +37,6 @@ export default function Repositories(): React.ReactElement { const classes = useStyles(); const [repos, setRepos] = useState([]); const [filteredRepos, setFilteredRepos] = useState([]); - const [, setAuth] = useState(true); const [isLoading, setIsLoading] = useState(false); const [isError, setIsError] = useState(false); const [errorMessage, setErrorMessage] = useState(''); @@ -49,16 +48,22 @@ export default function Repositories(): React.ReactElement { navigate(`/dashboard/repo/${repoId}`, { replace: true }); useEffect(() => { - getRepos( - setIsLoading, - (repos: RepoView[]) => { - setRepos(repos); - setFilteredRepos(repos); - }, - setAuth, - setIsError, - setErrorMessage, - ); + const load = async () => { + setIsLoading(true); + const result = await getRepos(); + if (result.success && result.data) { + setRepos(result.data); + setFilteredRepos(result.data); + } else if (result.status === 401) { + navigate('/login', { replace: true }); + return; + } else { + setIsError(true); + setErrorMessage(result.message || 'Failed to load repositories'); + } + setIsLoading(false); + }; + load(); }, []); const refresh = async (repo: RepoView): Promise => { diff --git a/test/ui/errors.test.ts b/test/ui/errors.test.ts new file mode 100644 index 000000000..3ba2b2b37 --- /dev/null +++ b/test/ui/errors.test.ts @@ -0,0 +1,223 @@ +import { describe, expect, it } from 'vitest'; +import { + getServiceError, + formatErrorMessage, + errorResult, + successResult, +} from '../../src/ui/services/errors'; + +describe('errors utility functions', () => { + describe('getServiceError', () => { + it('extracts status and message from axios error response', () => { + const error = { + response: { + status: 404, + data: { + message: 'Not found', + }, + }, + }; + + const result = getServiceError(error, 'Fallback message'); + + expect(result).toEqual({ + status: 404, + message: 'Not found', + }); + }); + + it('uses error.message when response message is not available', () => { + const error = { + message: 'Network error', + }; + + const result = getServiceError(error, 'Fallback message'); + + expect(result).toEqual({ + status: undefined, + message: 'Network error', + }); + }); + + it('uses fallback message when no message is available', () => { + const error = {}; + + const result = getServiceError(error, 'Fallback message'); + + expect(result).toEqual({ + status: undefined, + message: 'Fallback message', + }); + }); + + it('ignores empty string response messages', () => { + const error = { + response: { + status: 500, + data: { + message: ' ', + }, + }, + message: 'Server error', + }; + + const result = getServiceError(error, 'Fallback message'); + + expect(result).toEqual({ + status: 500, + message: 'Server error', + }); + }); + + it('ignores non-string response messages', () => { + const error = { + response: { + status: 400, + data: { + message: { error: 'Bad request' }, + }, + }, + message: 'Bad request error', + }; + + const result = getServiceError(error, 'Fallback message'); + + expect(result).toEqual({ + status: 400, + message: 'Bad request error', + }); + }); + }); + + describe('formatErrorMessage', () => { + it('formats message with status code', () => { + const result = formatErrorMessage('Error loading data', 404, 'Not found'); + + expect(result).toBe('Error loading data: 404 Not found'); + }); + + it('formats message without status code', () => { + const result = formatErrorMessage('Error loading data', undefined, 'Network error'); + + expect(result).toBe('Error loading data: Network error'); + }); + + it('handles status code 0', () => { + const result = formatErrorMessage('Error', 0, 'Connection refused'); + + expect(result).toBe('Error: Connection refused'); + }); + }); + + describe('errorResult', () => { + it('creates error result from axios error', () => { + const error = { + response: { + status: 403, + data: { + message: 'Forbidden', + }, + }, + }; + + const result = errorResult(error, 'Failed to access resource'); + + expect(result).toEqual({ + success: false, + status: 403, + message: 'Forbidden', + }); + }); + + it('creates error result with fallback message', () => { + const error = {}; + + const result = errorResult(error, 'Something went wrong'); + + expect(result).toEqual({ + success: false, + status: undefined, + message: 'Something went wrong', + }); + }); + + it('preserves type parameter', () => { + const error = { + message: 'Error', + }; + + const result = errorResult<{ data: string }>(error, 'Failed'); + + expect(result).toEqual({ + success: false, + status: undefined, + message: 'Error', + }); + }); + }); + + describe('successResult', () => { + it('creates success result without data', () => { + const result = successResult(); + + expect(result).toEqual({ + success: true, + }); + }); + + it('creates success result with data', () => { + const data = { id: '123', name: 'test' }; + const result = successResult(data); + + expect(result).toEqual({ + success: true, + data: { id: '123', name: 'test' }, + }); + }); + + it('creates success result with null data', () => { + const result = successResult(null); + + expect(result).toEqual({ + success: true, + data: null, + }); + }); + + it('creates success result with 0 as data', () => { + const result = successResult(0); + + expect(result).toEqual({ + success: true, + data: 0, + }); + }); + + it('creates success result with false as data', () => { + const result = successResult(false); + + expect(result).toEqual({ + success: true, + data: false, + }); + }); + + it('creates success result with empty string as data', () => { + const result = successResult(''); + + expect(result).toEqual({ + success: true, + data: '', + }); + }); + + it('does not include data key when undefined', () => { + const result = successResult(undefined); + + expect(result).toEqual({ + success: true, + }); + expect('data' in result).toBe(false); + }); + }); +}); diff --git a/test/ui/git-push.test.ts b/test/ui/git-push.test.ts index c7eb59258..ec6029635 100644 --- a/test/ui/git-push.test.ts +++ b/test/ui/git-push.test.ts @@ -1,11 +1,23 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import axios from 'axios'; -import { authorisePush, cancelPush, rejectPush } from '../../src/ui/services/git-push'; +import { + getPush, + getPushes, + authorisePush, + cancelPush, + rejectPush, +} from '../../src/ui/services/git-push'; + +const { axiosMock } = vi.hoisted(() => { + const axiosFn = vi.fn() as any; + axiosFn.post = vi.fn(); + + return { + axiosMock: axiosFn, + }; +}); vi.mock('axios', () => ({ - default: { - post: vi.fn(), - }, + default: axiosMock, })); vi.mock('../../src/ui/services/apiConfig', () => ({ @@ -20,94 +32,242 @@ vi.mock('../../src/ui/services/auth', () => ({ processAuthError: vi.fn(), })); -describe('git-push service action errors', () => { +describe('git-push service', () => { beforeEach(() => { vi.clearAllMocks(); }); - it('returns success for authorise action', async () => { - vi.mocked(axios.post).mockResolvedValue({ data: {} } as any); + describe('getPush', () => { + it('returns push data with diff step on success', async () => { + const pushData = { + id: 'push-123', + steps: [ + { stepName: 'diff', data: 'some diff' }, + { stepName: 'validate', data: 'validation data' }, + ], + }; - const result = await authorisePush('push-123', [{ label: 'LGTM', checked: true }]); + axiosMock.mockResolvedValue({ data: pushData }); + + const result = await getPush('push-123'); + + expect(result.success).toBe(true); + expect(result.data).toEqual({ + ...pushData, + diff: { stepName: 'diff', data: 'some diff' }, + }); + expect(axiosMock).toHaveBeenCalledWith( + 'http://localhost:8080/api/v1/push/push-123', + expect.any(Object), + ); + }); - expect(result).toEqual({ success: true }); - expect(axios.post).toHaveBeenCalledWith( - 'http://localhost:8080/api/v1/push/push-123/authorise', - { - params: { - attestation: [{ label: 'LGTM', checked: true }], + it('returns error result when getPush fails', async () => { + axiosMock.mockRejectedValue({ + response: { + status: 404, + data: { + message: 'Push not found', + }, }, - }, - expect.any(Object), - ); + }); + + const result = await getPush('push-123'); + + expect(result).toEqual({ + success: false, + status: 404, + message: 'Push not found', + }); + }); + + it('uses fallback message when error has no response data', async () => { + axiosMock.mockRejectedValue(new Error('Network error')); + + const result = await getPush('push-123'); + + expect(result).toEqual({ + success: false, + status: undefined, + message: 'Network error', + }); + }); }); - it('returns backend not-logged-in message for authorise 401 errors', async () => { - vi.mocked(axios.post).mockRejectedValue({ - response: { - status: 401, - data: { - message: 'Not logged in', - }, - }, + describe('getPushes', () => { + it('returns array of pushes on success with default query', async () => { + const pushesData = [ + { id: 'push-1', steps: [] }, + { id: 'push-2', steps: [] }, + ]; + + axiosMock.mockResolvedValue({ data: pushesData }); + + const result = await getPushes(); + + expect(result.success).toBe(true); + expect(result.data).toEqual(pushesData); + expect(axiosMock).toHaveBeenCalledWith( + 'http://localhost:8080/api/v1/push?blocked=true&canceled=false&authorised=false&rejected=false', + expect.any(Object), + ); + }); + + it('returns array of pushes with custom query params', async () => { + const pushesData = [{ id: 'push-1', steps: [] }]; + + axiosMock.mockResolvedValue({ data: pushesData }); + + const result = await getPushes({ + blocked: false, + canceled: true, + authorised: true, + rejected: false, + }); + + expect(result.success).toBe(true); + expect(result.data).toEqual(pushesData); + expect(axiosMock).toHaveBeenCalledWith( + 'http://localhost:8080/api/v1/push?blocked=false&canceled=true&authorised=true&rejected=false', + expect.any(Object), + ); }); - const result = await authorisePush('push-123', []); + it('returns error result when getPushes fails', async () => { + axiosMock.mockRejectedValue({ + response: { + status: 500, + data: { + message: 'Internal server error', + }, + }, + }); + + const result = await getPushes(); - expect(result).toEqual({ - success: false, - status: 401, - message: 'Not logged in', + expect(result).toEqual({ + success: false, + status: 500, + message: 'Internal server error', + }); }); }); - it('returns backend message for reject 403 errors', async () => { - vi.mocked(axios.post).mockRejectedValue({ - response: { - status: 403, - data: { - message: 'User alice is not authorised to reject changes on this project', + describe('authorisePush', () => { + it('returns success for authorise action', async () => { + axiosMock.post.mockResolvedValue({ data: {} } as any); + + const result = await authorisePush('push-123', [{ label: 'LGTM', checked: true }]); + + expect(result).toEqual({ success: true }); + expect(axiosMock.post).toHaveBeenCalledWith( + 'http://localhost:8080/api/v1/push/push-123/authorise', + { + params: { + attestation: [{ label: 'LGTM', checked: true }], + }, }, - }, + expect.any(Object), + ); }); - const result = await rejectPush('push-456'); + it('returns backend not-logged-in message for authorise 401 errors', async () => { + axiosMock.post.mockRejectedValue({ + response: { + status: 401, + data: { + message: 'Not logged in', + }, + }, + }); + + const result = await authorisePush('push-123', []); - expect(result).toEqual({ - success: false, - status: 403, - message: 'User alice is not authorised to reject changes on this project', + expect(result).toEqual({ + success: false, + status: 401, + message: 'Not logged in', + }); }); }); - it('returns backend message for cancel 401 errors', async () => { - vi.mocked(axios.post).mockRejectedValue({ - response: { - status: 401, - data: { - message: 'Not logged in', + describe('rejectPush', () => { + it('returns success for reject action', async () => { + axiosMock.post.mockResolvedValue({ data: {} } as any); + + const result = await rejectPush('push-456'); + + expect(result).toEqual({ success: true }); + expect(axiosMock.post).toHaveBeenCalledWith( + 'http://localhost:8080/api/v1/push/push-456/reject', + {}, + expect.any(Object), + ); + }); + + it('returns backend message for reject 403 errors', async () => { + axiosMock.post.mockRejectedValue({ + response: { + status: 403, + data: { + message: 'User alice is not authorised to reject changes on this project', + }, }, - }, + }); + + const result = await rejectPush('push-456'); + + expect(result).toEqual({ + success: false, + status: 403, + message: 'User alice is not authorised to reject changes on this project', + }); }); - const result = await cancelPush('push-789'); + it('falls back to thrown error message when response payload is missing', async () => { + axiosMock.post.mockRejectedValue(new Error('network timeout')); + + const result = await rejectPush('push-999'); - expect(result).toEqual({ - success: false, - status: 401, - message: 'Not logged in', + expect(result).toEqual({ + success: false, + status: undefined, + message: 'network timeout', + }); }); }); - it('falls back to thrown error message when response payload is missing', async () => { - vi.mocked(axios.post).mockRejectedValue(new Error('network timeout')); + describe('cancelPush', () => { + it('returns success for cancel action', async () => { + axiosMock.post.mockResolvedValue({ data: {} } as any); - const result = await rejectPush('push-999'); + const result = await cancelPush('push-789'); - expect(result).toEqual({ - success: false, - status: undefined, - message: 'network timeout', + expect(result).toEqual({ success: true }); + expect(axiosMock.post).toHaveBeenCalledWith( + 'http://localhost:8080/api/v1/push/push-789/cancel', + {}, + expect.any(Object), + ); + }); + + it('returns backend message for cancel 401 errors', async () => { + axiosMock.post.mockRejectedValue({ + response: { + status: 401, + data: { + message: 'Not logged in', + }, + }, + }); + + const result = await cancelPush('push-789'); + + expect(result).toEqual({ + success: false, + status: 401, + message: 'Not logged in', + }); }); }); }); diff --git a/test/ui/repo.test.ts b/test/ui/repo.test.ts index 4269a80cf..9f176e54a 100644 --- a/test/ui/repo.test.ts +++ b/test/ui/repo.test.ts @@ -1,7 +1,14 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { addUser, getRepo } from '../../src/ui/services/repo'; +import { + addUser, + deleteUser, + getRepo, + getRepos, + addRepo, + deleteRepo, +} from '../../src/ui/services/repo'; -const { axiosMock, processAuthErrorMock } = vi.hoisted(() => { +const { axiosMock } = vi.hoisted(() => { const axiosFn = vi.fn() as any; axiosFn.get = vi.fn(); axiosFn.post = vi.fn(); @@ -10,7 +17,6 @@ const { axiosMock, processAuthErrorMock } = vi.hoisted(() => { return { axiosMock: axiosFn, - processAuthErrorMock: vi.fn(() => 'Failed to authorize user: Not logged in.'), }; }); @@ -23,7 +29,7 @@ vi.mock('../../src/ui/services/auth.js', () => ({ withCredentials: true, headers: {}, })), - processAuthError: processAuthErrorMock, + processAuthError: vi.fn(), })); vi.mock('../../src/ui/services/apiConfig', () => ({ @@ -35,7 +41,20 @@ describe('repo service error handling', () => { vi.clearAllMocks(); }); - it('sets detailed error message when getRepo fails with non-401 status', async () => { + it('returns data on successful getRepo', async () => { + const repoData = { + name: 'test-repo', + project: 'org', + url: 'https://example.com/org/test-repo.git', + }; + axiosMock.mockResolvedValue({ data: repoData }); + + const result = await getRepo('repo-1'); + + expect(result).toEqual({ success: true, data: repoData }); + }); + + it('returns error result when getRepo fails with non-401 status', async () => { axiosMock.mockRejectedValue({ response: { status: 403, @@ -45,22 +64,16 @@ describe('repo service error handling', () => { }, }); - const setIsLoading = vi.fn(); - const setRepo = vi.fn(); - const setAuth = vi.fn(); - const setIsError = vi.fn(); - const setErrorMessage = vi.fn(); - - await getRepo(setIsLoading, setRepo, setAuth, setIsError, setErrorMessage, 'repo-1'); + const result = await getRepo('repo-1'); - expect(setIsError).toHaveBeenCalledWith(true); - expect(setErrorMessage).toHaveBeenCalledWith( - 'Error fetching repo: 403 User alice not authorised on this repository', - ); - expect(setAuth).not.toHaveBeenCalledWith(false); + expect(result).toEqual({ + success: false, + status: 403, + message: 'User alice not authorised on this repository', + }); }); - it('uses processAuthError when getRepo fails with 401 status', async () => { + it('returns error result when getRepo fails with 401 status', async () => { axiosMock.mockRejectedValue({ response: { status: 401, @@ -70,18 +83,25 @@ describe('repo service error handling', () => { }, }); - const setIsLoading = vi.fn(); - const setRepo = vi.fn(); - const setAuth = vi.fn(); - const setIsError = vi.fn(); - const setErrorMessage = vi.fn(); + const result = await getRepo('repo-1'); - await getRepo(setIsLoading, setRepo, setAuth, setIsError, setErrorMessage, 'repo-1'); + expect(result).toEqual({ + success: false, + status: 401, + message: 'Not logged in', + }); + }); + + it('returns fallback message when response payload is missing', async () => { + axiosMock.mockRejectedValue(new Error('network timeout')); - expect(setIsError).toHaveBeenCalledWith(true); - expect(setAuth).toHaveBeenCalledWith(false); - expect(processAuthErrorMock).toHaveBeenCalled(); - expect(setErrorMessage).toHaveBeenCalledWith('Failed to authorize user: Not logged in.'); + const result = await getRepo('repo-1'); + + expect(result).toEqual({ + success: false, + status: undefined, + message: 'network timeout', + }); }); it('throws backend message when addUser patch request fails', async () => { @@ -106,3 +126,255 @@ describe('repo service error handling', () => { await expect(addUser('repo-1', 'bob', 'authorise')).rejects.toThrow('User bob not found'); }); }); + +describe('repo service additional functions', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('getRepos', () => { + it('returns sorted repos on success', async () => { + const reposData = [ + { name: 'zebra-repo', project: 'org', url: 'https://example.com/org/zebra-repo.git' }, + { name: 'alpha-repo', project: 'org', url: 'https://example.com/org/alpha-repo.git' }, + ]; + + axiosMock.mockResolvedValue({ data: reposData }); + + const result = await getRepos(); + + expect(result.success).toBe(true); + expect(result.data).toEqual([ + { name: 'alpha-repo', project: 'org', url: 'https://example.com/org/alpha-repo.git' }, + { name: 'zebra-repo', project: 'org', url: 'https://example.com/org/zebra-repo.git' }, + ]); + }); + + it('passes query parameters correctly', async () => { + axiosMock.mockResolvedValue({ data: [] }); + + await getRepos({ active: true }); + + expect(axiosMock).toHaveBeenCalledWith( + 'http://localhost:8080/api/v1/repo?active=true', + expect.any(Object), + ); + }); + + it('returns error result when getRepos fails', async () => { + axiosMock.mockRejectedValue({ + response: { + status: 500, + data: { + message: 'Database connection failed', + }, + }, + }); + + const result = await getRepos(); + + expect(result).toEqual({ + success: false, + status: 500, + message: 'Database connection failed', + }); + }); + + it('uses fallback message when error has no response data', async () => { + axiosMock.mockRejectedValue(new Error('Connection timeout')); + + const result = await getRepos(); + + expect(result).toEqual({ + success: false, + status: undefined, + message: 'Connection timeout', + }); + }); + }); + + describe('addRepo', () => { + it('returns created repo on success', async () => { + const newRepo = { + name: 'new-repo', + project: 'org', + url: 'https://example.com/org/new-repo.git', + }; + + axiosMock.post.mockResolvedValue({ data: { ...newRepo, id: 'repo-123' } }); + + const result = await addRepo(newRepo as any); + + expect(result.success).toBe(true); + expect(result.data).toEqual({ ...newRepo, id: 'repo-123' }); + expect(axiosMock.post).toHaveBeenCalledWith( + 'http://localhost:8080/api/v1/repo', + newRepo, + expect.any(Object), + ); + }); + + it('returns error result when addRepo fails', async () => { + const newRepo = { + name: 'duplicate-repo', + project: 'org', + url: 'https://example.com/org/duplicate-repo.git', + }; + + axiosMock.post.mockRejectedValue({ + response: { + status: 409, + data: { + message: 'Repository already exists', + }, + }, + }); + + const result = await addRepo(newRepo as any); + + expect(result).toEqual({ + success: false, + status: 409, + message: 'Repository already exists', + }); + }); + }); + + describe('addUser', () => { + it('successfully adds user when not duplicate', async () => { + axiosMock.get.mockResolvedValue({ + data: { + users: { + canAuthorise: ['alice'], + canPush: ['bob'], + }, + }, + }); + + axiosMock.patch.mockResolvedValue({ data: {} }); + + await expect(addUser('repo-1', 'charlie', 'authorise')).resolves.toBeUndefined(); + + expect(axiosMock.patch).toHaveBeenCalledWith( + 'http://localhost:8080/api/v1/repo/repo-1/user/authorise', + { username: 'charlie' }, + expect.any(Object), + ); + }); + + it('throws DupUserValidationError when user already has the role', async () => { + axiosMock.get.mockResolvedValue({ + data: { + users: { + canAuthorise: ['alice'], + canPush: ['bob'], + }, + }, + }); + + await expect(addUser('repo-1', 'alice', 'authorise')).rejects.toThrow( + 'Duplicate user can not be added', + ); + + expect(axiosMock.patch).not.toHaveBeenCalled(); + }); + + it('checks canPush list for push action', async () => { + axiosMock.get.mockResolvedValue({ + data: { + users: { + canAuthorise: [], + canPush: ['bob'], + }, + }, + }); + + await expect(addUser('repo-1', 'bob', 'push')).rejects.toThrow( + 'Duplicate user can not be added', + ); + }); + + it('throws error from canAddUser validation failure', async () => { + axiosMock.get.mockRejectedValue({ + response: { + status: 404, + data: { + message: 'Repository not found', + }, + }, + }); + + await expect(addUser('repo-1', 'charlie', 'authorise')).rejects.toThrow( + 'Repository not found', + ); + }); + }); + + describe('deleteUser', () => { + it('successfully deletes user', async () => { + axiosMock.delete.mockResolvedValue({ data: {} }); + + await expect(deleteUser('alice', 'repo-1', 'authorise')).resolves.toBeUndefined(); + + expect(axiosMock.delete).toHaveBeenCalledWith( + 'http://localhost:8080/api/v1/repo/repo-1/user/authorise/alice', + expect.any(Object), + ); + }); + + it('throws error when deleteUser fails', async () => { + axiosMock.delete.mockRejectedValue({ + response: { + status: 404, + data: { + message: 'User not found in repository', + }, + }, + }); + + await expect(deleteUser('charlie', 'repo-1', 'authorise')).rejects.toThrow( + 'User not found in repository', + ); + }); + + it('throws fallback message when error has no response data', async () => { + axiosMock.delete.mockRejectedValue(new Error('Network error')); + + await expect(deleteUser('alice', 'repo-1', 'push')).rejects.toThrow('Network error'); + }); + }); + + describe('deleteRepo', () => { + it('successfully deletes repository', async () => { + axiosMock.delete.mockResolvedValue({ data: {} }); + + await expect(deleteRepo('repo-1')).resolves.toBeUndefined(); + + expect(axiosMock.delete).toHaveBeenCalledWith( + 'http://localhost:8080/api/v1/repo/repo-1/delete', + expect.any(Object), + ); + }); + + it('throws error when deleteRepo fails', async () => { + axiosMock.delete.mockRejectedValue({ + response: { + status: 403, + data: { + message: 'Insufficient permissions to delete repository', + }, + }, + }); + + await expect(deleteRepo('repo-1')).rejects.toThrow( + 'Insufficient permissions to delete repository', + ); + }); + + it('throws fallback message when error has no response data', async () => { + axiosMock.delete.mockRejectedValue(new Error('Connection refused')); + + await expect(deleteRepo('repo-1')).rejects.toThrow('Connection refused'); + }); + }); +}); diff --git a/test/ui/user.test.ts b/test/ui/user.test.ts new file mode 100644 index 000000000..3cbb236c4 --- /dev/null +++ b/test/ui/user.test.ts @@ -0,0 +1,265 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { getUser, getUsers, updateUser } from '../../src/ui/services/user'; + +const { axiosMock } = vi.hoisted(() => { + const axiosFn = vi.fn() as any; + axiosFn.post = vi.fn(); + + return { + axiosMock: axiosFn, + }; +}); + +vi.mock('axios', () => ({ + default: axiosMock, +})); + +vi.mock('../../src/ui/services/apiConfig', () => ({ + getBaseUrl: vi.fn(async () => 'http://localhost:8080'), + getApiV1BaseUrl: vi.fn(async () => 'http://localhost:8080/api/v1'), +})); + +vi.mock('../../src/ui/services/auth', () => ({ + getAxiosConfig: vi.fn(() => ({ + withCredentials: true, + headers: {}, + })), + processAuthError: vi.fn((error) => `Auth error: ${error?.response?.data?.message || 'Unknown'}`), +})); + +describe('user service', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('getUser', () => { + it('fetches current user profile when no id is provided', async () => { + const userData = { id: 'user-1', username: 'alice', email: 'alice@example.com' }; + const setUser = vi.fn(); + const setIsLoading = vi.fn(); + + axiosMock.mockResolvedValue({ data: userData }); + + await getUser(setIsLoading, setUser); + + expect(axiosMock).toHaveBeenCalledWith( + 'http://localhost:8080/api/auth/profile', + expect.any(Object), + ); + expect(setUser).toHaveBeenCalledWith(userData); + expect(setIsLoading).toHaveBeenCalledWith(false); + }); + + it('fetches specific user when id is provided', async () => { + const userData = { id: 'user-2', username: 'bob', email: 'bob@example.com' }; + const setUser = vi.fn(); + const setIsLoading = vi.fn(); + + axiosMock.mockResolvedValue({ data: userData }); + + await getUser(setIsLoading, setUser, undefined, undefined, 'user-2'); + + expect(axiosMock).toHaveBeenCalledWith( + 'http://localhost:8080/api/v1/user/user-2', + expect.any(Object), + ); + expect(setUser).toHaveBeenCalledWith(userData); + expect(setIsLoading).toHaveBeenCalledWith(false); + }); + + it('handles 401 auth errors', async () => { + const setAuth = vi.fn(); + const setErrorMessage = vi.fn(); + const setIsLoading = vi.fn(); + + axiosMock.mockRejectedValue({ + response: { + status: 401, + data: { + message: 'Session expired', + }, + }, + }); + + await getUser(setIsLoading, undefined, setAuth, setErrorMessage); + + expect(setAuth).toHaveBeenCalledWith(false); + expect(setErrorMessage).toHaveBeenCalledWith('Auth error: Session expired'); + expect(setIsLoading).toHaveBeenCalledWith(false); + }); + + it('handles non-401 errors with formatted message', async () => { + const setErrorMessage = vi.fn(); + const setIsLoading = vi.fn(); + + axiosMock.mockRejectedValue({ + response: { + status: 404, + data: { + message: 'User not found', + }, + }, + }); + + await getUser(setIsLoading, undefined, undefined, setErrorMessage); + + expect(setErrorMessage).toHaveBeenCalledWith('Error fetching user: 404 User not found'); + expect(setIsLoading).toHaveBeenCalledWith(false); + }); + + it('handles errors without status code', async () => { + const setErrorMessage = vi.fn(); + const setIsLoading = vi.fn(); + + axiosMock.mockRejectedValue(new Error('Network timeout')); + + await getUser(setIsLoading, undefined, undefined, setErrorMessage); + + expect(setErrorMessage).toHaveBeenCalledWith('Error fetching user: Network timeout'); + expect(setIsLoading).toHaveBeenCalledWith(false); + }); + + it('works with minimal callbacks provided', async () => { + const userData = { id: 'user-1', username: 'alice', email: 'alice@example.com' }; + + axiosMock.mockResolvedValue({ data: userData }); + + await expect(getUser()).resolves.toBeUndefined(); + }); + }); + + describe('getUsers', () => { + it('fetches all users successfully', async () => { + const usersData = [ + { id: 'user-1', username: 'alice', email: 'alice@example.com' }, + { id: 'user-2', username: 'bob', email: 'bob@example.com' }, + ]; + const setUsers = vi.fn(); + const setIsLoading = vi.fn(); + const setAuth = vi.fn(); + const setErrorMessage = vi.fn(); + + axiosMock.mockResolvedValue({ data: usersData }); + + await getUsers(setIsLoading, setUsers, setAuth, setErrorMessage); + + expect(axiosMock).toHaveBeenCalledWith( + 'http://localhost:8080/api/v1/user', + expect.any(Object), + ); + expect(setIsLoading).toHaveBeenCalledWith(true); + expect(setUsers).toHaveBeenCalledWith(usersData); + expect(setIsLoading).toHaveBeenCalledWith(false); + }); + + it('handles 401 errors', async () => { + const setUsers = vi.fn(); + const setIsLoading = vi.fn(); + const setAuth = vi.fn(); + const setErrorMessage = vi.fn(); + + axiosMock.mockRejectedValue({ + response: { + status: 401, + data: { + message: 'Not authenticated', + }, + }, + }); + + await getUsers(setIsLoading, setUsers, setAuth, setErrorMessage); + + expect(setAuth).toHaveBeenCalledWith(false); + expect(setErrorMessage).toHaveBeenCalledWith('Auth error: Not authenticated'); + expect(setIsLoading).toHaveBeenCalledWith(false); + }); + + it('handles non-401 errors', async () => { + const setUsers = vi.fn(); + const setIsLoading = vi.fn(); + const setAuth = vi.fn(); + const setErrorMessage = vi.fn(); + + axiosMock.mockRejectedValue({ + response: { + status: 500, + data: { + message: 'Database error', + }, + }, + }); + + await getUsers(setIsLoading, setUsers, setAuth, setErrorMessage); + + expect(setErrorMessage).toHaveBeenCalledWith('Error fetching users: 500 Database error'); + expect(setIsLoading).toHaveBeenCalledWith(false); + }); + + it('sets loading to false even when error occurs', async () => { + const setUsers = vi.fn(); + const setIsLoading = vi.fn(); + const setAuth = vi.fn(); + const setErrorMessage = vi.fn(); + + axiosMock.mockRejectedValue(new Error('Network error')); + + await getUsers(setIsLoading, setUsers, setAuth, setErrorMessage); + + expect(setIsLoading).toHaveBeenCalledWith(true); + expect(setIsLoading).toHaveBeenCalledWith(false); + }); + }); + + describe('updateUser', () => { + it('successfully updates user', async () => { + const userData = { id: 'user-1', username: 'alice', email: 'alice@example.com' }; + const setErrorMessage = vi.fn(); + const setIsLoading = vi.fn(); + + axiosMock.post.mockResolvedValue({ data: {} }); + + await updateUser(userData as any, setErrorMessage, setIsLoading); + + expect(axiosMock.post).toHaveBeenCalledWith( + 'http://localhost:8080/api/auth/gitAccount', + userData, + expect.any(Object), + ); + expect(setErrorMessage).not.toHaveBeenCalled(); + expect(setIsLoading).not.toHaveBeenCalled(); + }); + + it('handles update errors', async () => { + const userData = { id: 'user-1', username: 'alice', email: 'alice@example.com' }; + const setErrorMessage = vi.fn(); + const setIsLoading = vi.fn(); + + axiosMock.post.mockRejectedValue({ + response: { + status: 400, + data: { + message: 'Invalid email format', + }, + }, + }); + + await updateUser(userData as any, setErrorMessage, setIsLoading); + + expect(setErrorMessage).toHaveBeenCalledWith('Error updating user: 400 Invalid email format'); + expect(setIsLoading).toHaveBeenCalledWith(false); + }); + + it('handles errors without status code', async () => { + const userData = { id: 'user-1', username: 'alice', email: 'alice@example.com' }; + const setErrorMessage = vi.fn(); + const setIsLoading = vi.fn(); + + axiosMock.post.mockRejectedValue(new Error('Connection failed')); + + await updateUser(userData as any, setErrorMessage, setIsLoading); + + expect(setErrorMessage).toHaveBeenCalledWith('Error updating user: Connection failed'); + expect(setIsLoading).toHaveBeenCalledWith(false); + }); + }); +}); From 76559e354f0104c67fff7f3f2b4f4537b215cf2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C4=86ori=C4=87?= Date: Fri, 13 Feb 2026 18:32:22 +0100 Subject: [PATCH 3/7] fix: ensure loading state is updated on 401 responses in multiple components --- src/ui/views/PushDetails/PushDetails.tsx | 1 + src/ui/views/PushRequests/components/PushesTable.tsx | 1 + src/ui/views/RepoDetails/RepoDetails.tsx | 11 +++++++++++ src/ui/views/RepoList/Components/Repositories.tsx | 1 + 4 files changed, 14 insertions(+) diff --git a/src/ui/views/PushDetails/PushDetails.tsx b/src/ui/views/PushDetails/PushDetails.tsx index 9320adc76..66354a9dd 100644 --- a/src/ui/views/PushDetails/PushDetails.tsx +++ b/src/ui/views/PushDetails/PushDetails.tsx @@ -54,6 +54,7 @@ const Dashboard: React.FC = () => { if (result.success && result.data) { setPush(result.data); } else if (result.status === 401) { + setIsLoading(false); navigate('/login', { replace: true }); return; } else { diff --git a/src/ui/views/PushRequests/components/PushesTable.tsx b/src/ui/views/PushRequests/components/PushesTable.tsx index 8f029de79..4e174be07 100644 --- a/src/ui/views/PushRequests/components/PushesTable.tsx +++ b/src/ui/views/PushRequests/components/PushesTable.tsx @@ -53,6 +53,7 @@ const PushesTable: React.FC = (props) => { if (result.success && result.data) { setPushes(result.data); } else if (result.status === 401) { + setIsLoading(false); navigate('/login', { replace: true }); return; } else if (props.handleError) { diff --git a/src/ui/views/RepoDetails/RepoDetails.tsx b/src/ui/views/RepoDetails/RepoDetails.tsx index 4a6822a50..65a21dabe 100644 --- a/src/ui/views/RepoDetails/RepoDetails.tsx +++ b/src/ui/views/RepoDetails/RepoDetails.tsx @@ -61,6 +61,7 @@ const RepoDetails: React.FC = () => { if (result.success && result.data) { setRepo(result.data); } else if (result.status === 401) { + setIsLoading(false); navigate('/login', { replace: true }); return; } else { @@ -84,6 +85,11 @@ const RepoDetails: React.FC = () => { const result = await getRepo(repoId); if (result.success && result.data) { setRepo(result.data); + } else if (result.status === 401) { + navigate('/login', { replace: true }); + } else { + setIsError(true); + setErrorMessage(result.message || 'Failed to refresh repository data'); } }; @@ -97,6 +103,11 @@ const RepoDetails: React.FC = () => { const result = await getRepo(repoId); if (result.success && result.data) { setRepo(result.data); + } else if (result.status === 401) { + navigate('/login', { replace: true }); + } else { + setIsError(true); + setErrorMessage(result.message || 'Failed to refresh repository data'); } }; diff --git a/src/ui/views/RepoList/Components/Repositories.tsx b/src/ui/views/RepoList/Components/Repositories.tsx index e4a4e89b7..5c7ca5ea6 100644 --- a/src/ui/views/RepoList/Components/Repositories.tsx +++ b/src/ui/views/RepoList/Components/Repositories.tsx @@ -55,6 +55,7 @@ export default function Repositories(): React.ReactElement { setRepos(result.data); setFilteredRepos(result.data); } else if (result.status === 401) { + setIsLoading(false); navigate('/login', { replace: true }); return; } else { From 5cf407a52024068238c0eb066131a03588549058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C4=86ori=C4=87?= Date: Wed, 18 Feb 2026 12:29:33 +0100 Subject: [PATCH 4/7] fix: improve error handling in service responses and update tests --- src/ui/services/errors.ts | 4 +++- src/ui/services/git-push.ts | 11 +++++++---- src/ui/types.ts | 13 ++++++++++++- test/ui/errors.test.ts | 8 ++++---- 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/ui/services/errors.ts b/src/ui/services/errors.ts index 12554240e..19bddb562 100644 --- a/src/ui/services/errors.ts +++ b/src/ui/services/errors.ts @@ -14,7 +14,9 @@ export const getServiceError = ( const message = typeof responseMessage === 'string' && responseMessage.trim().length > 0 ? responseMessage - : error?.message || fallbackMessage; + : status + ? `Unknown error occurred, response code: ${status}` + : error?.message || fallbackMessage; return { status, message }; }; diff --git a/src/ui/services/git-push.ts b/src/ui/services/git-push.ts index ccae6d7ef..3e1812619 100644 --- a/src/ui/services/git-push.ts +++ b/src/ui/services/git-push.ts @@ -11,9 +11,12 @@ const getPush = async (id: string): Promise> => { try { const response = await axios(url, getAxiosConfig()); - const data: Action & { diff?: Step } = response.data; - data.diff = data.steps.find((x: Step) => x.stepName === 'diff'); - return successResult(data as PushActionView); + const data: Action = response.data; + const actionView: PushActionView = { + ...data, + diff: data.steps.find((x: Step) => x.stepName === 'diff')!, + }; + return successResult(actionView); } catch (error: any) { return errorResult(error, 'Failed to load push'); } @@ -33,7 +36,7 @@ const getPushes = async ( try { const response = await axios(url.toString(), getAxiosConfig()); - return successResult(response.data as PushActionView[]); + return successResult(response.data as unknown as PushActionView[]); } catch (error: any) { return errorResult(error, 'Failed to load pushes'); } diff --git a/src/ui/types.ts b/src/ui/types.ts index 342208d56..2ccde6877 100644 --- a/src/ui/types.ts +++ b/src/ui/types.ts @@ -4,7 +4,18 @@ import { Repo } from '../db/types'; import { Attestation } from '../proxy/processors/types'; import { Question } from '../config/generated/config'; -export interface PushActionView extends Action { +type ActionMethods = + | 'addStep' + | 'getLastStep' + | 'setCommit' + | 'setBranch' + | 'setMessage' + | 'setAllowPush' + | 'setAutoApproval' + | 'setAutoRejection' + | 'continue'; + +export interface PushActionView extends Omit { diff: Step; } diff --git a/test/ui/errors.test.ts b/test/ui/errors.test.ts index 3ba2b2b37..f5a101721 100644 --- a/test/ui/errors.test.ts +++ b/test/ui/errors.test.ts @@ -50,7 +50,7 @@ describe('errors utility functions', () => { }); }); - it('ignores empty string response messages', () => { + it('uses response code when response message is empty', () => { const error = { response: { status: 500, @@ -65,11 +65,11 @@ describe('errors utility functions', () => { expect(result).toEqual({ status: 500, - message: 'Server error', + message: 'Unknown error occurred, response code: 500', }); }); - it('ignores non-string response messages', () => { + it('uses response code when response message is non-string', () => { const error = { response: { status: 400, @@ -84,7 +84,7 @@ describe('errors utility functions', () => { expect(result).toEqual({ status: 400, - message: 'Bad request error', + message: 'Unknown error occurred, response code: 400', }); }); }); From 7290c33db5a3440030140f33793d4ed20852cd88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C4=86ori=C4=87?= Date: Thu, 19 Feb 2026 13:54:32 +0100 Subject: [PATCH 5/7] fix: enhance error handling for user removal and repository deletion --- src/ui/views/RepoDetails/RepoDetails.tsx | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/ui/views/RepoDetails/RepoDetails.tsx b/src/ui/views/RepoDetails/RepoDetails.tsx index 65a21dabe..0b5250030 100644 --- a/src/ui/views/RepoDetails/RepoDetails.tsx +++ b/src/ui/views/RepoDetails/RepoDetails.tsx @@ -81,7 +81,13 @@ const RepoDetails: React.FC = () => { const removeUser = async (userToRemove: string, action: 'authorise' | 'push') => { if (!repoId) return; - await deleteUser(userToRemove, repoId, action); + try { + await deleteUser(userToRemove, repoId, action); + } catch (err: any) { + setIsError(true); + setErrorMessage(err.message || 'Failed to remove user'); + return; + } const result = await getRepo(repoId); if (result.success && result.data) { setRepo(result.data); @@ -94,7 +100,13 @@ const RepoDetails: React.FC = () => { }; const removeRepository = async (id: string) => { - await deleteRepo(id); + try { + await deleteRepo(id); + } catch (err: any) { + setIsError(true); + setErrorMessage(err.message || 'Failed to delete repository'); + return; + } navigate('/dashboard/repo', { replace: true }); }; From 960bad5cc3829d85609a851a40a320c483f7a559 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C4=86ori=C4=87?= Date: Fri, 20 Feb 2026 17:31:31 +0100 Subject: [PATCH 6/7] feat: add ErrorBoundary component to handle errors in Dashboard layout --- .../ErrorBoundary/ErrorBoundary.tsx | 174 ++++++++++++++++++ src/ui/layouts/Dashboard.tsx | 23 ++- 2 files changed, 187 insertions(+), 10 deletions(-) create mode 100644 src/ui/components/ErrorBoundary/ErrorBoundary.tsx diff --git a/src/ui/components/ErrorBoundary/ErrorBoundary.tsx b/src/ui/components/ErrorBoundary/ErrorBoundary.tsx new file mode 100644 index 000000000..0e84fa599 --- /dev/null +++ b/src/ui/components/ErrorBoundary/ErrorBoundary.tsx @@ -0,0 +1,174 @@ +import React, { Component, ErrorInfo, PropsWithChildren, ReactNode, useState } from 'react'; +import Paper from '@material-ui/core/Paper'; +import Typography from '@material-ui/core/Typography'; +import Button from '@material-ui/core/Button'; +import Collapse from '@material-ui/core/Collapse'; +import { makeStyles } from '@material-ui/core/styles'; + +const IS_DEV = process.env.NODE_ENV !== 'production'; + +const useStyles = makeStyles((theme) => ({ + wrapper: { + display: 'flex', + alignItems: 'center', + justifyContent: 'center', + height: '100%', + minHeight: '60vh', + padding: theme.spacing(2), + }, + root: { + padding: theme.spacing(4), + borderLeft: `4px solid ${theme.palette.error.main}`, + maxWidth: 560, + width: '100%', + }, + title: { + color: theme.palette.error.main, + marginBottom: theme.spacing(1), + }, + message: { + marginBottom: theme.spacing(2), + color: theme.palette.text.secondary, + }, + hint: { + marginBottom: theme.spacing(2), + color: theme.palette.text.secondary, + fontStyle: 'italic', + }, + actions: { + display: 'flex', + gap: theme.spacing(1), + alignItems: 'center', + marginBottom: theme.spacing(1), + }, + stack: { + marginTop: theme.spacing(2), + padding: theme.spacing(2), + backgroundColor: theme.palette.grey[100], + borderRadius: theme.shape.borderRadius, + overflowX: 'auto', + fontSize: '0.75rem', + fontFamily: 'monospace', + whiteSpace: 'pre-wrap', + wordBreak: 'break-word', + }, + devBadge: { + display: 'inline-block', + marginBottom: theme.spacing(2), + padding: '2px 8px', + backgroundColor: theme.palette.warning.main, + color: theme.palette.warning.contrastText, + borderRadius: theme.shape.borderRadius, + fontSize: '0.7rem', + fontWeight: 700, + letterSpacing: '0.05em', + textTransform: 'uppercase', + }, +})); + +const ProdFallback = ({ reset }: { reset: () => void }) => { + const classes = useStyles(); + return ( +
+ + + Something went wrong + + + An unexpected error occurred. Please try again — if the problem persists, contact your + administrator. + +
+ + +
+ +
+ ); +}; + +const DevFallback = ({ + error, + name, + reset, +}: { + error: Error; + name?: string; + reset: () => void; +}) => { + const classes = useStyles(); + const [showDetails, setShowDetails] = useState(false); + const context = name ? ` in ${name}` : ''; + + return ( +
+ +
dev
+ + Something went wrong{context} + + + {error.message} + +
+ + {error.stack && ( + + )} +
+ {error.stack && ( + + 
{error.stack}
+ + )} + +
+ ); +}; + +type Props = PropsWithChildren<{ + name?: string; + fallback?: (error: Error, reset: () => void) => ReactNode; + onError?: (error: Error, errorInfo: ErrorInfo) => void; +}>; + +type State = { error: Error | undefined }; + +export class ErrorBoundary extends Component { + state: State = { error: undefined }; + + static getDerivedStateFromError(error: Error): State { + return { error }; + } + + componentDidCatch(error: Error, errorInfo: ErrorInfo) { + this.props.onError?.(error, errorInfo); + console.error('ErrorBoundary caught:', error, errorInfo); + } + + reset = () => this.setState({ error: undefined }); + + render() { + const { error } = this.state; + const { children, fallback, name } = this.props; + + if (error) { + if (fallback) return fallback(error, this.reset); + return IS_DEV ? ( + + ) : ( + + ); + } + + return children; + } +} diff --git a/src/ui/layouts/Dashboard.tsx b/src/ui/layouts/Dashboard.tsx index 3666a2bd1..96a28ad66 100644 --- a/src/ui/layouts/Dashboard.tsx +++ b/src/ui/layouts/Dashboard.tsx @@ -13,6 +13,7 @@ import { UserContext } from '../context'; import { getUser } from '../services/user'; import { Route as RouteType } from '../types'; import { PublicUser } from '../../db/types'; +import { ErrorBoundary } from '../components/ErrorBoundary/ErrorBoundary'; interface DashboardProps { [key: string]: any; @@ -95,16 +96,18 @@ const Dashboard: React.FC = ({ ...rest }) => { />
- {isMapRoute() ? ( -
{switchRoutes}
- ) : ( - <> -
-
{switchRoutes}
-
-
- - )} + + {isMapRoute() ? ( +
{switchRoutes}
+ ) : ( + <> +
+
{switchRoutes}
+
+
+ + )} +
From fa6c123191d0202a5e51f4a7387644a499324827 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C4=86ori=C4=87?= Date: Mon, 23 Feb 2026 12:45:31 +0100 Subject: [PATCH 7/7] fix: improve error handling and logging in ErrorBoundary and auth services --- src/ui/components/ErrorBoundary/ErrorBoundary.tsx | 4 +++- src/ui/services/auth.ts | 13 +++++++++++-- src/ui/views/PushDetails/PushDetails.tsx | 3 +-- src/ui/views/User/UserProfile.tsx | 3 +-- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/ui/components/ErrorBoundary/ErrorBoundary.tsx b/src/ui/components/ErrorBoundary/ErrorBoundary.tsx index 0e84fa599..d13b05821 100644 --- a/src/ui/components/ErrorBoundary/ErrorBoundary.tsx +++ b/src/ui/components/ErrorBoundary/ErrorBoundary.tsx @@ -151,7 +151,9 @@ export class ErrorBoundary extends Component { componentDidCatch(error: Error, errorInfo: ErrorInfo) { this.props.onError?.(error, errorInfo); - console.error('ErrorBoundary caught:', error, errorInfo); + if (IS_DEV) { + console.error('ErrorBoundary caught:', error, errorInfo); + } } reset = () => this.setState({ error: undefined }); diff --git a/src/ui/services/auth.ts b/src/ui/services/auth.ts index f9f4346c5..1111d920b 100644 --- a/src/ui/services/auth.ts +++ b/src/ui/services/auth.ts @@ -11,6 +11,8 @@ interface AxiosConfig { }; } +const IS_DEV = process.env.NODE_ENV !== 'production'; + /** * Gets the current user's information */ @@ -20,10 +22,17 @@ export const getUserInfo = async (): Promise => { const response = await fetch(`${baseUrl}/api/auth/profile`, { credentials: 'include', // Sends cookies }); - if (!response.ok) throw new Error(`Failed to fetch user info: ${response.statusText}`); + if (!response.ok) { + if (response.status === 401) { + return null; + } + throw new Error(`Failed to fetch user info: ${response.statusText}`); + } return await response.json(); } catch (error) { - console.error('Error fetching user info:', error); + if (IS_DEV) { + console.warn('Error fetching user info:', error); + } return null; } }; diff --git a/src/ui/views/PushDetails/PushDetails.tsx b/src/ui/views/PushDetails/PushDetails.tsx index 66354a9dd..2ed1a6e51 100644 --- a/src/ui/views/PushDetails/PushDetails.tsx +++ b/src/ui/views/PushDetails/PushDetails.tsx @@ -27,7 +27,6 @@ import { AttestationFormData, PushActionView } from '../../types'; import { trimPrefixRefsHeads, trimTrailingDotGit } from '../../../db/helper'; import { generateEmailLink, getGitProvider } from '../../utils'; import UserLink from '../../components/UserLink/UserLink'; -import Danger from '../../components/Typography/Danger'; const Dashboard: React.FC = () => { const { id } = useParams<{ id: string }>(); @@ -97,7 +96,7 @@ const Dashboard: React.FC = () => { }; if (isLoading) return
Loading...
; - if (isError) return {message || 'Something went wrong ...'}; + if (isError) throw new Error(message || 'Something went wrong ...'); if (!push) return
No push data found
; let headerData: { title: string; color: CardHeaderColor } = { diff --git a/src/ui/views/User/UserProfile.tsx b/src/ui/views/User/UserProfile.tsx index 49b9b6053..bfd54286e 100644 --- a/src/ui/views/User/UserProfile.tsx +++ b/src/ui/views/User/UserProfile.tsx @@ -16,7 +16,6 @@ import { LogoGithubIcon } from '@primer/octicons-react'; import CloseRounded from '@material-ui/icons/CloseRounded'; import { Check, Save } from '@material-ui/icons'; import { TextField, Theme } from '@material-ui/core'; -import Danger from '../../components/Typography/Danger'; const useStyles = makeStyles((theme: Theme) => ({ root: { @@ -58,7 +57,7 @@ export default function UserProfile(): React.ReactElement { return ; } - if (errorMessage) return {errorMessage}; + if (errorMessage) throw new Error(errorMessage); if (!user) return
No user data available
;