Skip to content

Commit

Permalink
feat: improve response for not found data
Browse files Browse the repository at this point in the history
- change the status code from 404 to 200 (OK)
- use makeRequest function to treat error from endpoint in the frontend
- show error message in SectionError component

Part of #784
  • Loading branch information
Francisco2002 committed Jan 23, 2025
1 parent 529f6c3 commit 8451117
Show file tree
Hide file tree
Showing 25 changed files with 96 additions and 77 deletions.
2 changes: 1 addition & 1 deletion backend/kernelCI_app/views/buildDetailsView.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def get(self, request, build_id):
records = query.select()
if not records:
return create_error_response(
error_message="Build not found", status_code=HTTPStatus.NOT_FOUND
error_message="Build not found", status_code=HTTPStatus.OK
)

return JsonResponse(records[0])
2 changes: 1 addition & 1 deletion backend/kernelCI_app/views/buildTestsView.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def get(self, request, build_id):
if not result:
return create_error_response(
error_message="Tests not found for this build",
status_code=HTTPStatus.NOT_FOUND,
status_code=HTTPStatus.OK,
)

return JsonResponse(list(result), safe=False)
2 changes: 1 addition & 1 deletion backend/kernelCI_app/views/issueDetailsBuildsView.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def get(

if not builds_data:
return create_error_response(
error_message="Builds not found", status_code=HTTPStatus.NOT_FOUND
error_message="Builds not found", status_code=HTTPStatus.OK
)

return JsonResponse(builds_data, safe=False)
2 changes: 1 addition & 1 deletion backend/kernelCI_app/views/issueDetailsTestsView.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def get(

if not tests_data:
return create_error_response(
error_message="Tests not found", status_code=HTTPStatus.NOT_FOUND
error_message="Tests not found", status_code=HTTPStatus.OK
)

return JsonResponse(tests_data, safe=False)
2 changes: 1 addition & 1 deletion backend/kernelCI_app/views/issueDetailsView.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def get(

if not issue_data:
return create_error_response(
error_message="Issue not found", status_code=HTTPStatus.NOT_FOUND
error_message="Issue not found", status_code=HTTPStatus.OK
)

return JsonResponse(issue_data)
4 changes: 2 additions & 2 deletions backend/kernelCI_app/views/issuesView.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def get(
if len(test_issues) == 0:
return create_error_response(
error_message="No issues were found for this test",
status_code=HTTPStatus.NOT_FOUND,
status_code=HTTPStatus.OK,
)

return JsonResponse(test_issues, safe=False)
Expand All @@ -97,7 +97,7 @@ def get(
if len(build_issues) == 0:
return create_error_response(
error_message="No issues were found for this build",
status_code=HTTPStatus.NOT_FOUND,
status_code=HTTPStatus.OK,
)

return JsonResponse(build_issues, safe=False)
Expand Down
2 changes: 1 addition & 1 deletion backend/kernelCI_app/views/logDownloaderView.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def get(self, request):

if not parsed_data['log_files']:
return create_error_response(
error_message="No log files found", status_code=HTTPStatus.NOT_FOUND
error_message="No log files found", status_code=HTTPStatus.OK
)

return JsonResponse(parsed_data)
2 changes: 1 addition & 1 deletion backend/kernelCI_app/views/testDetailsView.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def get(self, _request, test_id: str | None):

if len(response) == 0:
return create_error_response(
error_message="Test not found", status_code=HTTPStatus.NOT_FOUND
error_message="Test not found", status_code=HTTPStatus.OK
)

return JsonResponse(response, safe=False)
2 changes: 1 addition & 1 deletion backend/kernelCI_app/views/treeCommitsHistory.py
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ def get(self, request, commit_hash):

if not rows:
return create_error_response(
error_message="History of tree commits not found", status_code=HTTPStatus.NOT_FOUND
error_message="History of tree commits not found", status_code=HTTPStatus.OK
)

self._process_rows(rows)
Expand Down
2 changes: 1 addition & 1 deletion backend/kernelCI_app/views/treeDetailsBootsView.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def get(self, request, commit_hash: str | None):

if len(rows) == 0:
return create_error_response(
error_message="Tree not found", status_code=HTTPStatus.NOT_FOUND
error_message="Boots not found", status_code=HTTPStatus.OK
)

try:
Expand Down
2 changes: 1 addition & 1 deletion backend/kernelCI_app/views/treeDetailsBuildsView.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def get(self, request, commit_hash: str | None):

if len(rows) == 0:
return create_error_response(
error_message="Tree not found", status_code=HTTPStatus.NOT_FOUND
error_message="Builds not found", status_code=HTTPStatus.OK
)

try:
Expand Down
2 changes: 1 addition & 1 deletion backend/kernelCI_app/views/treeDetailsSummaryView.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def get(self, request, commit_hash: str | None):

if len(rows) == 0:
return create_error_response(
error_message="Tree not found", status_code=HTTPStatus.NOT_FOUND
error_message="Tree not found", status_code=HTTPStatus.OK
)

self.filters = FilterParams(request)
Expand Down
2 changes: 1 addition & 1 deletion backend/kernelCI_app/views/treeDetailsTestsView.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def get(self, request, commit_hash: str | None):

if len(rows) == 0:
return create_error_response(
error_message="Tree not found", status_code=HTTPStatus.NOT_FOUND
error_message="Tests not found", status_code=HTTPStatus.OK
)

try:
Expand Down
2 changes: 1 addition & 1 deletion backend/kernelCI_app/views/treeDetailsView.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ def get(self, request, commit_hash: str | None):

if len(rows) == 0:
return create_error_response(
error_message="Tree not found", status_code=HTTPStatus.NOT_FOUND
error_message="Tree not found", status_code=HTTPStatus.OK
)

self._sanitize_rows(rows)
Expand Down
2 changes: 1 addition & 1 deletion backend/kernelCI_app/views/treeView.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def get(self, request):

if not checkouts:
return create_error_response(
error_message="Trees not found", status_code=HTTPStatus.NOT_FOUND
error_message="Trees not found", status_code=HTTPStatus.OK
)

serializer = TreeSerializer(checkouts, many=True)
Expand Down
19 changes: 12 additions & 7 deletions dashboard/src/api/buildDetails.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@ import type { TBuildDetails } from '@/types/tree/BuildDetails';

import type { TIssue } from '@/types/general';

import http from './api';
import { makeRequest } from './commonRequest';

const fetchTreeDetailData = async (buildId: string): Promise<TBuildDetails> => {
const res = await http.get(`/api/build/${buildId}`);
const fetchBuildDetailData = async (
buildId: string,
): Promise<TBuildDetails> => {
const res = await makeRequest<TBuildDetails & { _timestamp: string }>(
`/api/build/${buildId}`,
);

const { _timestamp, ...data } = res.data;
const { _timestamp, ...data } = res;
data.timestamp = _timestamp;
return data;
};
Expand All @@ -20,13 +24,14 @@ export const useBuildDetails = (
): UseQueryResult<TBuildDetails> => {
return useQuery({
queryKey: ['treeData', buildId],
queryFn: () => fetchTreeDetailData(buildId),
queryFn: () => fetchBuildDetailData(buildId),
});
};

const fetchBuildIssues = async (buildId: string): Promise<TIssue[]> => {
const res = await http.get<TIssue[]>(`/api/build/${buildId}/issues`);
return res.data;
const data = await makeRequest<TIssue[]>(`/api/build/${buildId}/issues`);

return data;
};

export const useBuildIssues = (buildId: string): UseQueryResult<TIssue[]> => {
Expand Down
6 changes: 3 additions & 3 deletions dashboard/src/api/buildTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import { useQuery } from '@tanstack/react-query';

import type { TestHistory } from '@/types/general';

import http from './api';
import { makeRequest } from './commonRequest';

const fetchBuildTestsData = async (buildId: string): Promise<TestHistory[]> => {
const res = await http.get(`/api/build/${buildId}/tests`);
const data = await makeRequest<TestHistory[]>(`/api/build/${buildId}/tests`);

return res.data;
return data;
};

export const useBuildTests = (
Expand Down
10 changes: 6 additions & 4 deletions dashboard/src/api/commitHistory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import type {

import { mapFiltersKeysToBackendCompatible } from '@/utils/utils';

import { getTargetFilter, type TFilter } from '@/types/general';
import { getTargetFilter } from '@/types/general';
import type { TFilter } from '@/types/general';

import http from './api';
import { makeRequest } from './commonRequest';

const fetchCommitHistory = async (
commitHash: string,
Expand All @@ -32,13 +33,14 @@ const fetchCommitHistory = async (
...filtersFormatted,
};

const res = await http.get<TTreeCommitHistoryResponse>(
const data = await makeRequest<TTreeCommitHistoryResponse>(
`/api/tree/${commitHash}/commits`,
{
params,
},
);
return res.data;

return data;
};

export const useCommitHistory = ({
Expand Down
16 changes: 9 additions & 7 deletions dashboard/src/api/issueDetails.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,17 @@ import type { TErrorWithStatus, TIssueDetails } from '@/types/issueDetails';

import type { BuildsTableBuild, TestHistory } from '@/types/general';

import http from './api';
import { makeRequest } from './commonRequest';

const fetchIssueDetailsData = async (
issueId: string,
versionNumber: string,
): Promise<TIssueDetails> => {
const res = await http.get(`/api/issue/${issueId}/version/${versionNumber}`);
const res = await makeRequest<TIssueDetails & { _timestamp: string }>(
`/api/issue/${issueId}/version/${versionNumber}`,
);

const { _timestamp, ...data } = res.data;
const { _timestamp, ...data } = res;
data.timestamp = _timestamp;
return data;
};
Expand All @@ -32,11 +34,11 @@ const fetchIssueDetailsTests = async (
issueId: string,
versionNumber: string,
): Promise<TestHistory[]> => {
const res = await http.get(
const data = await makeRequest<TestHistory[]>(
`/api/issue/${issueId}/version/${versionNumber}/tests`,
);

return res.data;
return data;
};

export const useIssueDetailsTests = (
Expand All @@ -53,11 +55,11 @@ const fetchIssueDetailsBuilds = async (
issueId: string,
versionNumber: string,
): Promise<BuildsTableBuild[]> => {
const res = await http.get(
const data = await makeRequest<BuildsTableBuild[]>(
`/api/issue/${issueId}/version/${versionNumber}/builds`,
);

return res.data;
return data;
};

export const useIssueDetailsBuilds = (
Expand Down
11 changes: 6 additions & 5 deletions dashboard/src/api/testDetails.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ import type { TTestDetails } from '@/types/tree/TestDetails';

import type { TIssue } from '@/types/general';

import http from './api';
import { makeRequest } from './commonRequest';

const fetchTestDetails = async (testId: string): Promise<TTestDetails> => {
const res = await http.get<TTestDetails>(`/api/test/${testId}`);
return res.data;
const data = await makeRequest<TTestDetails>(`/api/test/${testId}`);
return data;
};

export const useTestDetails = (
Expand All @@ -22,8 +22,9 @@ export const useTestDetails = (
};

const fetchTestIssues = async (testId: string): Promise<TIssue[]> => {
const res = await http.get<TIssue[]>(`/api/test/${testId}/issues`);
return res.data;
const data = await makeRequest<TIssue[]>(`/api/test/${testId}/issues`);

return data;
};

export const useTestIssues = (testId: string): UseQueryResult<TIssue[]> => {
Expand Down
14 changes: 8 additions & 6 deletions dashboard/src/api/treeDetails.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { getTargetFilter, type TFilter } from '@/types/general';

import { mapFiltersKeysToBackendCompatible } from '@/utils/utils';

import http from './api';
import { makeRequest } from './commonRequest';

type TreeSearchParameters = {
origin: string;
Expand Down Expand Up @@ -72,11 +72,12 @@ const fetchTreeDetails = async ({
tests: `/api/tree/${treeId}/tests`,
summary: `/api/tree/${treeId}/summary`,
};
const res = await http.get<TreeDetailsFullData>(urlTable[variant], {
params: params,

const data = await makeRequest<TreeDetailsFullData>(urlTable[variant], {
params,
});

return res.data;
return data;
};

type TreeDetailsResponse<T extends keyof TreeDetailsResponseTable> =
Expand Down Expand Up @@ -127,12 +128,13 @@ export const useTreeDetails = <T extends TreeDetailsVariants>({
};

const fetchLogFiles = async (logUrl: string): Promise<BuildCountsResponse> => {
const res = await http.get<BuildCountsResponse>(`/api/log-downloader/`, {
const data = await makeRequest<BuildCountsResponse>(`/api/log-downloader/`, {
params: {
log_download_url: logUrl,
},
});
return res.data;

return data;
};

type Config = {
Expand Down
13 changes: 8 additions & 5 deletions dashboard/src/components/BuildDetails/BuildDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,12 @@ const BuildDetails = ({
historyState,
}: BuildDetailsProps): JSX.Element => {
const searchParams = useSearch({ from: '/build/$buildId' });
const { data, isLoading, status } = useBuildDetails(buildId ?? '');
const { data: issueData, status: issueStatus } = useBuildIssues(
buildId ?? '',
);
const { data, isLoading, status, error } = useBuildDetails(buildId ?? '');
const {
data: issueData,
status: issueStatus,
error: issueError,
} = useBuildIssues(buildId ?? '');

const { formatMessage } = useIntl();

Expand Down Expand Up @@ -233,7 +235,7 @@ const BuildDetails = ({
customError={
<MemoizedSectionError
isLoading={isLoading}
errorMessage={formatMessage({ id: 'buildDetails.failedToFetch' })}
errorMessage={error?.message}
emptyLabel={'global.error'}
/>
}
Expand All @@ -252,6 +254,7 @@ const BuildDetails = ({
<IssueSection
data={issueData}
status={issueStatus}
error={issueError?.message}
historyState={historyState}
previousSearch={searchParams}
/>
Expand Down
Loading

0 comments on commit 8451117

Please sign in to comment.