IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Correlations: Return 200 instead of 404 for empt correlation lists (#55242)

Browse Source 
* return 200 instead of 404 for empty correlations list

* handle empty list response & improve consistency
This commit is contained in:
Giordano Ricci 2022-09-20 11:13:33 +01:00 committed by GitHub
parent d014a3a09b
commit d815e2cb56
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 82 additions and 112 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
pkg/services/correlations/api.go
View File

@ -224,7 +224,7 @@ func (s *CorrelationsService) getCorrelationHandler(c *models.ReqContext) respon
return response.Error(http.StatusNotFound, "Source data source not found", err)
}
return response.Error(http.StatusInternalServerError, "Failed to update correlation", err)
return response.Error(http.StatusInternalServerError, "Failed to get correlation", err)
}
return response.JSON(http.StatusOK, correlation)
@ -270,7 +270,7 @@ func (s *CorrelationsService) getCorrelationsBySourceUIDHandler(c *models.ReqCon
return response.Error(http.StatusNotFound, "Source data source not found", err)
}
return response.Error(http.StatusInternalServerError, "Failed to update correlation", err)
return response.Error(http.StatusInternalServerError, "Failed to get correlations", err)
}
return response.JSON(http.StatusOK, correlations)
@ -309,7 +309,7 @@ func (s *CorrelationsService) getCorrelationsHandler(c *models.ReqContext) respo
return response.Error(http.StatusNotFound, "No correlation found", err)
}
return response.Error(http.StatusInternalServerError, "Failed to update correlation", err)
return response.Error(http.StatusInternalServerError, "Failed to get correlations", err)
}
return response.JSON(http.StatusOK, correlations)

8
pkg/services/correlations/database.go
View File

@ -192,10 +192,6 @@ func (s CorrelationsService) getCorrelationsBySourceUID(ctx context.Context, cmd
return []Correlation{}, err
}
if len(correlations) == 0 {
return []Correlation{}, ErrCorrelationNotFound
}
return correlations, nil
}
@ -209,10 +205,6 @@ func (s CorrelationsService) getCorrelations(ctx context.Context, cmd GetCorrela
return []Correlation{}, err
}
if len(correlations) == 0 {
return []Correlation{}, ErrCorrelationNotFound
}
return correlations, nil
}

16
pkg/tests/api/correlations/correlations_read_test.go
View File

@ -42,21 +42,21 @@ func TestIntegrationReadCorrelation(t *testing.T) {
t.Run("Get all correlations", func(t *testing.T) {
// Running this here before creating a correlation in order to test this path.
t.Run("If no correlation exists it should return 404", func(t *testing.T) {
t.Run("If no correlation exists it should return 200", func(t *testing.T) {
res := ctx.Get(GetParams{
url: "/api/datasources/correlations",
user: adminUser,
})
require.Equal(t, http.StatusNotFound, res.StatusCode)
require.Equal(t, http.StatusOK, res.StatusCode)
responseBody, err := io.ReadAll(res.Body)
require.NoError(t, err)
var response errorResponseBody
var response []correlations.Correlation
err = json.Unmarshal(responseBody, &response)
require.NoError(t, err)
require.Equal(t, "No correlation found", response.Message)
require.Len(t, response, 0)
require.NoError(t, res.Body.Close())
})
@ -183,21 +183,21 @@ func TestIntegrationReadCorrelation(t *testing.T) {
require.NoError(t, res.Body.Close())
})
t.Run("If no correlation exists it should return 404", func(t *testing.T) {
t.Run("If no correlation exists it should return 200", func(t *testing.T) {
res := ctx.Get(GetParams{
url: fmt.Sprintf("/api/datasources/uid/%s/correlations", dsWithoutCorrelations.Uid),
user: adminUser,
})
require.Equal(t, http.StatusNotFound, res.StatusCode)
require.Equal(t, http.StatusOK, res.StatusCode)
responseBody, err := io.ReadAll(res.Body)
require.NoError(t, err)
var response errorResponseBody
var response []correlations.Correlation
err = json.Unmarshal(responseBody, &response)
require.NoError(t, err)
require.Equal(t, "No correlation found", response.Message)
require.Len(t, response, 0)
require.NoError(t, res.Body.Close())
})

7
public/app/features/correlations/CorrelationsPage.test.tsx
View File

@ -119,11 +119,8 @@ const renderWithContext = async (
},
fetch: (options: BackendSrvRequest) => {
return new Observable((s) => {
if (correlations.length) {
s.next(merge(createFetchResponse({ url: options.url, data: correlations })));
} else {
s.error(merge(createFetchError({ config: { url: options.url }, status: 404 })));
}
s.next(merge(createFetchResponse({ url: options.url, data: correlations })));
s.complete();
});
},

97
public/app/features/correlations/CorrelationsPage.tsx
View File

@ -4,6 +4,7 @@ import React, { memo, useCallback, useEffect, useMemo, useState } from 'react';
import { CellProps, SortByFn } from 'react-table';
import { GrafanaTheme2 } from '@grafana/data';
import { isFetchError } from '@grafana/runtime';
import { Badge, Button, DeleteButton, HorizontalGroup, LoadingPlaceholder, useStyles2, Alert } from '@grafana/ui';
import { Page } from 'app/core/components/Page/Page';
import { contextSrv } from 'app/core/core';
@ -14,7 +15,6 @@ import { AddCorrelationForm } from './Forms/AddCorrelationForm';
import { EditCorrelationForm } from './Forms/EditCorrelationForm';
import { EmptyCorrelationsCTA } from './components/EmptyCorrelationsCTA';
import { Column, Table } from './components/Table';
import { RemoveCorrelationParams } from './types';
import { CorrelationData, useCorrelations } from './useCorrelations';
const sortDatasource: SortByFn<CorrelationData> = (a, b, column) =>
@ -30,10 +30,13 @@ const loaderWrapper = css`
export default function CorrelationsPage() {
const navModel = useNavModel('correlations');
const [isAdding, setIsAdding] = useState(false);
const { remove, get } = useCorrelations();
const {
remove,
get: { execute: fetchCorrelations, ...get },
} = useCorrelations();
useEffect(() => {
get.execute();
fetchCorrelations();
// we only want to fetch data on first render
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
@ -41,21 +44,15 @@ export default function CorrelationsPage() {
const canWriteCorrelations = contextSrv.hasPermission(AccessControlAction.DataSourcesWrite);
const handleAdd = useCallback(() => {
get.execute();
fetchCorrelations();
setIsAdding(false);
}, [get]);
}, [fetchCorrelations]);
const handleUpdate = useCallback(() => {
get.execute();
}, [get]);
const handleRemove = useCallback<(params: RemoveCorrelationParams) => void>(
async (correlation) => {
await remove.execute(correlation);
get.execute();
},
[remove, get]
);
useEffect(() => {
if (!remove.error && !remove.loading && remove.value) {
fetchCorrelations();
}
}, [remove.error, remove.loading, remove.value, fetchCorrelations]);
const RowActions = useCallback(
({
@ -69,11 +66,11 @@ export default function CorrelationsPage() {
!readOnly && (
<DeleteButton
aria-label="delete correlation"
onConfirm={() => handleRemove({ sourceUID, uid })}
onConfirm={() => remove.execute({ sourceUID, uid })}
closeOnConfirm
/>
),
[handleRemove]
[remove]
);
const columns = useMemo<Array<Column<CorrelationData>>>(
@ -107,7 +104,7 @@ export default function CorrelationsPage() {
const data = useMemo(() => get.value, [get.value]);
const showEmptyListCTA = data?.length === 0 && !isAdding && (!get.error || get.error.status === 404);
const showEmptyListCTA = data?.length === 0 && !isAdding && !get.error;
return (
<Page navModel={navModel}>
@ -126,42 +123,42 @@ export default function CorrelationsPage() {
</HorizontalGroup>
</div>
{!data && get.loading && (
<div className={loaderWrapper}>
<LoadingPlaceholder text="loading..." />
</div>
)}
<div>
{!data && get.loading && (
<div className={loaderWrapper}>
<LoadingPlaceholder text="loading..." />
</div>
)}
{showEmptyListCTA && <EmptyCorrelationsCTA onClick={() => setIsAdding(true)} />}
{showEmptyListCTA && <EmptyCorrelationsCTA onClick={() => setIsAdding(true)} />}
{
// This error is not actionable, it'd be nice to have a recovery button
get.error && get.error.status !== 404 && (
<Alert severity="error" title="Error fetching correlation data" topSpacing={2}>
<HorizontalGroup>
{get.error.data.message ||
{
// This error is not actionable, it'd be nice to have a recovery button
get.error && (
<Alert severity="error" title="Error fetching correlation data" topSpacing={2}>
{(isFetchError(get.error) && get.error.data?.message) ||
'An unknown error occurred while fetching correlation data. Please try again.'}
</HorizontalGroup>
</Alert>
)
}
</Alert>
)
}
{isAdding && <AddCorrelationForm onClose={() => setIsAdding(false)} onCreated={handleAdd} />}
{isAdding && <AddCorrelationForm onClose={() => setIsAdding(false)} onCreated={handleAdd} />}
{data && data.length >= 1 && (
<Table
renderExpandedRow={({ target, source, ...correlation }) => (
<EditCorrelationForm
defaultValues={{ sourceUID: source.uid, ...correlation }}
onUpdated={handleUpdate}
readOnly={isSourceReadOnly({ source }) || !canWriteCorrelations}
/>
)}
columns={columns}
data={data}
getRowId={(correlation) => `${correlation.source.uid}-${correlation.uid}`}
/>
)}
{data && data.length >= 1 && (
<Table
renderExpandedRow={({ target, source, ...correlation }) => (
<EditCorrelationForm
defaultValues={{ sourceUID: source.uid, ...correlation }}
onUpdated={fetchCorrelations}
readOnly={isSourceReadOnly({ source }) || !canWriteCorrelations}
/>
)}
columns={columns}
data={data}
getRowId={(correlation) => `${correlation.source.uid}-${correlation.uid}`}
/>
)}
</div>
</Page.Contents>
</Page>
);

25
public/app/features/correlations/Forms/AddCorrelationForm.tsx
View File

@ -1,5 +1,5 @@
import { css } from '@emotion/css';
import React, { useCallback } from 'react';
import React, { useEffect } from 'react';
import { Controller } from 'react-hook-form';
import { DataSourceInstanceSettings, GrafanaTheme2 } from '@grafana/data';
@ -47,17 +47,17 @@ const withDsUID = (fn: Function) => (ds: DataSourceInstanceSettings) => fn(ds.ui
export const AddCorrelationForm = ({ onClose, onCreated }: Props) => {
const styles = useStyles2(getStyles);
const { create } = useCorrelations();
const {
create: { execute, loading, error, value },
} = useCorrelations();
const onSubmit = useCallback(
async (correlation) => {
await create.execute(correlation);
useEffect(() => {
if (!error && !loading && value) {
onCreated();
},
[create, onCreated]
);
}
}, [error, loading, value, onCreated]);
const { control, handleSubmit, register, errors } = useCorrelationForm<FormDTO>({ onSubmit });
const { control, handleSubmit, register, errors } = useCorrelationForm<FormDTO>({ onSubmit: execute });
return (
<PanelContainer className={styles.panelContainer}>
@ -108,12 +108,7 @@ export const AddCorrelationForm = ({ onClose, onCreated }: Props) => {
<CorrelationDetailsFormPart register={register} />
<HorizontalGroup justify="flex-end">
<Button
variant="primary"
icon={create.loading ? 'fa fa-spinner' : 'plus'}
type="submit"
disabled={create.loading}
>
<Button variant="primary" icon={loading ? 'fa fa-spinner' : 'plus'} type="submit" disabled={loading}>
Add
</Button>
</HorizontalGroup>

25
public/app/features/correlations/Forms/EditCorrelationForm.tsx
View File

@ -1,4 +1,4 @@
import React, { useCallback } from 'react';
import React, { useEffect } from 'react';
import { Button, HorizontalGroup } from '@grafana/ui';
@ -15,17 +15,17 @@ interface Props {
}
export const EditCorrelationForm = ({ onUpdated, defaultValues, readOnly = false }: Props) => {
const { update } = useCorrelations();
const {
update: { execute, loading, error, value },
} = useCorrelations();
const onSubmit = useCallback(
async (correlation) => {
await update.execute(correlation);
useEffect(() => {
if (!error && !loading && value) {
onUpdated();
},
[update, onUpdated]
);
}
}, [error, loading, value, onUpdated]);
const { handleSubmit, register } = useCorrelationForm<EditFormDTO>({ onSubmit, defaultValues });
const { handleSubmit, register } = useCorrelationForm<EditFormDTO>({ onSubmit: execute, defaultValues });
return (
<form onSubmit={readOnly ? (e) => e.preventDefault() : handleSubmit}>
@ -35,12 +35,7 @@ export const EditCorrelationForm = ({ onUpdated, defaultValues, readOnly = false
{!readOnly && (
<HorizontalGroup justify="flex-end">
<Button
variant="primary"
icon={update.loading ? 'fa fa-spinner' : 'save'}
type="submit"
disabled={update.loading}
>
<Button variant="primary" icon={loading ? 'fa fa-spinner' : 'save'} type="submit" disabled={loading}>
Save
</Button>
</HorizontalGroup>

10
public/app/features/correlations/useCorrelations.ts
View File

@ -1,9 +1,8 @@
import { useState } from 'react';
import { useAsyncFn } from 'react-use';
import { lastValueFrom } from 'rxjs';
import { DataSourceInstanceSettings } from '@grafana/data';
import { getDataSourceSrv, FetchResponse, FetchError } from '@grafana/runtime';
import { getDataSourceSrv, FetchResponse } from '@grafana/runtime';
import { useGrafana } from 'app/core/context/GrafanaContext';
import { Correlation, CreateCorrelationParams, RemoveCorrelationParams, UpdateCorrelationParams } from './types';
@ -32,17 +31,13 @@ function getData<T>(response: FetchResponse<T>) {
*/
export const useCorrelations = () => {
const { backend } = useGrafana();
const [error, setError] = useState<FetchError | null>(null);
const [getInfo, get] = useAsyncFn<() => Promise<CorrelationData[]>>(
() =>
lastValueFrom(
backend.fetch<Correlation[]>({ url: '/api/datasources/correlations', method: 'GET', showErrorAlert: false })
)
.then(getData, (e) => {
setError(e);
return [];
})
.then(getData)
.then(toEnrichedCorrelationsData),
[backend]
);
@ -78,7 +73,6 @@ export const useCorrelations = () => {
get: {
execute: get,
...getInfo,
error,
},
remove: {
execute: remove,