diff --git a/pkg/apis/provisioning/v0alpha1/types.go b/pkg/apis/provisioning/v0alpha1/types.go index 6996422fba6..9c069f96ad4 100644 --- a/pkg/apis/provisioning/v0alpha1/types.go +++ b/pkg/apis/provisioning/v0alpha1/types.go @@ -394,11 +394,14 @@ type TestResults struct { // Is the connection healthy Success bool `json:"success"` - // Error descriptions - Errors []string `json:"errors,omitempty"` + // Field related errors + Errors []ErrorDetails `json:"errors,omitempty"` +} - // Optional details - Details *common.Unstructured `json:"details,omitempty"` +type ErrorDetails struct { + Type metav1.CauseType `json:"type"` + Field string `json:"field,omitempty"` + Detail string `json:"detail,omitempty"` } // HistoryList is a list of versions of a resource diff --git a/pkg/apis/provisioning/v0alpha1/zz_generated.deepcopy.go b/pkg/apis/provisioning/v0alpha1/zz_generated.deepcopy.go index 8e09d703d09..2beb7403f6a 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.deepcopy.go @@ -27,6 +27,22 @@ func (in *Author) DeepCopy() *Author { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ErrorDetails) DeepCopyInto(out *ErrorDetails) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ErrorDetails. +func (in *ErrorDetails) DeepCopy() *ErrorDetails { + if in == nil { + return nil + } + out := new(ErrorDetails) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ExportJobOptions) DeepCopyInto(out *ExportJobOptions) { *out = *in @@ -849,13 +865,9 @@ func (in *TestResults) DeepCopyInto(out *TestResults) { out.TypeMeta = in.TypeMeta if in.Errors != nil { in, out := &in.Errors, &out.Errors - *out = make([]string, len(*in)) + *out = make([]ErrorDetails, len(*in)) copy(*out, *in) } - if in.Details != nil { - in, out := &in.Details, &out.Details - *out = (*in).DeepCopy() - } return } diff --git a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go index 184ea75003a..4f9483d09b7 100644 --- a/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go +++ b/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go @@ -15,6 +15,7 @@ import ( func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition { return map[string]common.OpenAPIDefinition{ "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.Author": schema_pkg_apis_provisioning_v0alpha1_Author(ref), + "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.ErrorDetails": schema_pkg_apis_provisioning_v0alpha1_ErrorDetails(ref), "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.ExportJobOptions": schema_pkg_apis_provisioning_v0alpha1_ExportJobOptions(ref), "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.FileItem": schema_pkg_apis_provisioning_v0alpha1_FileItem(ref), "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.FileList": schema_pkg_apis_provisioning_v0alpha1_FileList(ref), @@ -88,6 +89,38 @@ func schema_pkg_apis_provisioning_v0alpha1_Author(ref common.ReferenceCallback) } } +func schema_pkg_apis_provisioning_v0alpha1_ErrorDetails(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "type": { + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + "field": { + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + "detail": { + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + }, + Required: []string{"type"}, + }, + }, + } +} + func schema_pkg_apis_provisioning_v0alpha1_ExportJobOptions(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -1909,31 +1942,24 @@ func schema_pkg_apis_provisioning_v0alpha1_TestResults(ref common.ReferenceCallb }, "errors": { SchemaProps: spec.SchemaProps{ - Description: "Error descriptions", + Description: "Field related errors", Type: []string{"array"}, Items: &spec.SchemaOrArray{ Schema: &spec.Schema{ SchemaProps: spec.SchemaProps{ - Default: "", - Type: []string{"string"}, - Format: "", + Default: map[string]interface{}{}, + Ref: ref("github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.ErrorDetails"), }, }, }, }, }, - "details": { - SchemaProps: spec.SchemaProps{ - Description: "Optional details", - Ref: ref("github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1.Unstructured"), - }, - }, }, Required: []string{"code", "success"}, }, }, Dependencies: []string{ - "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1.Unstructured"}, + "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1.ErrorDetails"}, } } diff --git a/pkg/registry/apis/provisioning/controller/repository.go b/pkg/registry/apis/provisioning/controller/repository.go index 3274ed0928b..f4c7e09d7e9 100644 --- a/pkg/registry/apis/provisioning/controller/repository.go +++ b/pkg/registry/apis/provisioning/controller/repository.go @@ -271,18 +271,22 @@ func (rc *RepositoryController) runHealthCheck(ctx context.Context, repo reposit if err != nil { res = &provisioning.TestResults{ Success: false, - Errors: []string{ - "error running test repository", - err.Error(), - }, + Errors: []provisioning.ErrorDetails{{ + Detail: fmt.Sprintf("error running test repository: %s", err.Error()), + }}, } } healthStatus := provisioning.HealthStatus{ Healthy: res.Success, Checked: time.Now().UnixMilli(), - Message: res.Errors, } + for _, err := range res.Errors { + if err.Detail != "" { + healthStatus.Message = append(healthStatus.Message, err.Detail) + } + } + logger.Info("health check completed", "status", healthStatus) return healthStatus diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 7f8c70d497a..9db18a4e9be 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -1114,7 +1114,7 @@ func (b *APIBuilder) AsRepository(ctx context.Context, r *provisioning.Repositor return gogit.Clone(ctx, b.clonedir, r, opts, b.secrets) } - return repository.NewGitHub(ctx, r, b.ghFactory, b.secrets, webhookURL, cloneFn) + return repository.NewGitHub(ctx, r, b.ghFactory, b.secrets, webhookURL, cloneFn), nil default: return nil, fmt.Errorf("unknown repository type (%s)", r.Spec.Type) } diff --git a/pkg/registry/apis/provisioning/repository/github.go b/pkg/registry/apis/provisioning/repository/github.go index 7da5d961409..90b64584666 100644 --- a/pkg/registry/apis/provisioning/repository/github.go +++ b/pkg/registry/apis/provisioning/repository/github.go @@ -12,12 +12,11 @@ import ( "strings" "github.com/google/go-github/v70/github" + "github.com/google/uuid" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" - "github.com/google/uuid" - "github.com/grafana/grafana-app-sdk/logging" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" pgh "github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/github" @@ -57,18 +56,14 @@ func NewGitHub( secrets secrets.Service, webhookURL string, cloneFn CloneFn, -) (*githubRepository, error) { - owner, repo, err := parseOwnerRepo(config.Spec.GitHub.URL) - if err != nil { - return nil, err - } +) *githubRepository { + owner, repo, _ := parseOwnerRepo(config.Spec.GitHub.URL) token := config.Spec.GitHub.Token if token == "" { decrypted, err := secrets.Decrypt(ctx, config.Spec.GitHub.EncryptedToken) - if err != nil { - return nil, err + if err == nil { + token = string(decrypted) } - token = string(decrypted) } return &githubRepository{ config: config, @@ -78,7 +73,7 @@ func NewGitHub( owner: owner, repo: repo, cloneFn: cloneFn, - }, nil + } } func (r *githubRepository) Config() *provisioning.Repository { @@ -138,59 +133,48 @@ func parseOwnerRepo(giturl string) (owner string, repo string, err error) { return parts[1], parts[2], nil } -func fromError(err error, code int) *provisioning.TestResults { - statusErr, ok := err.(apierrors.APIStatus) - if ok { - s := statusErr.Status() - return &provisioning.TestResults{ - Code: int(s.Code), - Success: false, - Errors: []string{s.Message}, - } - } - return &provisioning.TestResults{ - Code: code, - Success: false, - Errors: []string{err.Error()}, - } -} - // Test implements provisioning.Repository. func (r *githubRepository) Test(ctx context.Context) (*provisioning.TestResults, error) { if err := r.gh.IsAuthenticated(ctx); err != nil { - return fromError(err, http.StatusUnauthorized), nil + return &provisioning.TestResults{ + Code: http.StatusBadRequest, + Success: false, + Errors: []provisioning.ErrorDetails{{ + Type: metav1.CauseTypeFieldValueInvalid, + Field: field.NewPath("spec", "github", "token").String(), + Detail: err.Error(), + }}}, nil } - owner, repo, err := parseOwnerRepo(r.config.Spec.GitHub.URL) + url := r.config.Spec.GitHub.URL + owner, repo, err := parseOwnerRepo(url) if err != nil { - return fromError(err, http.StatusBadRequest), nil + return fromFieldError(field.Invalid( + field.NewPath("spec", "github", "url"), url, err.Error())), nil } // FIXME: check token permissions ok, err := r.gh.RepoExists(ctx, owner, repo) if err != nil { - return fromError(err, http.StatusBadRequest), nil + return fromFieldError(field.Invalid( + field.NewPath("spec", "github", "url"), url, err.Error())), nil } if !ok { - return &provisioning.TestResults{ - Code: http.StatusBadRequest, - Success: false, - Errors: []string{"repository does not exist"}, - }, nil + return fromFieldError(field.NotFound( + field.NewPath("spec", "github", "url"), url)), nil } - ok, err = r.gh.BranchExists(ctx, r.owner, r.repo, r.config.Spec.GitHub.Branch) + branch := r.config.Spec.GitHub.Branch + ok, err = r.gh.BranchExists(ctx, r.owner, r.repo, branch) if err != nil { - return fromError(err, http.StatusBadRequest), nil + return fromFieldError(field.Invalid( + field.NewPath("spec", "github", "branch"), branch, err.Error())), nil } if !ok { - return &provisioning.TestResults{ - Code: http.StatusBadRequest, - Success: false, - Errors: []string{"branch does not exist"}, - }, nil + return fromFieldError(field.NotFound( + field.NewPath("spec", "github", "branch"), branch)), nil } return &provisioning.TestResults{ diff --git a/pkg/registry/apis/provisioning/repository/local.go b/pkg/registry/apis/provisioning/repository/local.go index d28663e4680..7c5a237a94c 100644 --- a/pkg/registry/apis/provisioning/repository/local.go +++ b/pkg/registry/apis/provisioning/repository/local.go @@ -146,36 +146,19 @@ func (r *localRepository) Validate() (fields field.ErrorList) { // Test implements provisioning.Repository. // NOTE: Validate has been called (and passed) before this function should be called func (r *localRepository) Test(ctx context.Context) (*provisioning.TestResults, error) { + path := field.NewPath("spec", "localhost", "path") if r.config.Spec.Local.Path == "" { - return &provisioning.TestResults{ - Code: http.StatusBadRequest, - Success: false, - Errors: []string{ - "no path is configured", - }, - }, nil + return fromFieldError(field.Required(path, "no path is configured")), nil } _, err := r.resolver.LocalPath(r.config.Spec.Local.Path) if err != nil { - return &provisioning.TestResults{ - Code: http.StatusBadRequest, - Success: false, - Errors: []string{ - err.Error(), - }, - }, nil + return fromFieldError(field.Invalid(path, r.config.Spec.Local.Path, err.Error())), nil } _, err = os.Stat(r.path) if errors.Is(err, os.ErrNotExist) { - return &provisioning.TestResults{ - Code: http.StatusBadRequest, - Success: false, - Errors: []string{ - fmt.Sprintf("directory not found: %s", r.config.Spec.Local.Path), - }, - }, nil + return fromFieldError(field.NotFound(path, r.config.Spec.Local.Path)), nil } return &provisioning.TestResults{ diff --git a/pkg/registry/apis/provisioning/repository/test.go b/pkg/registry/apis/provisioning/repository/test.go index 8ba064ea08c..76f5b4d4f67 100644 --- a/pkg/registry/apis/provisioning/repository/test.go +++ b/pkg/registry/apis/provisioning/repository/test.go @@ -6,6 +6,7 @@ import ( "net/http" "slices" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" @@ -26,10 +27,14 @@ func TestRepository(ctx context.Context, repo Repository) (*provisioning.TestRes rsp := &provisioning.TestResults{ Code: http.StatusUnprocessableEntity, // Invalid Success: false, - Errors: make([]string, len(errors)), + Errors: make([]provisioning.ErrorDetails, len(errors)), } - for i, v := range errors { - rsp.Errors[i] = v.Error() + for i, err := range errors { + rsp.Errors[i] = provisioning.ErrorDetails{ + Type: metav1.CauseType(err.Type), + Field: err.Field, + Detail: err.Detail, + } } return rsp, nil } @@ -85,3 +90,15 @@ func ValidateRepository(repo Repository) field.ErrorList { return list } + +func fromFieldError(err *field.Error) *provisioning.TestResults { + return &provisioning.TestResults{ + Code: http.StatusBadRequest, + Success: false, + Errors: []provisioning.ErrorDetails{{ + Type: metav1.CauseType(err.Type), + Field: err.Field, + Detail: err.Detail, + }}, + } +} diff --git a/pkg/registry/apis/provisioning/repository/test_test.go b/pkg/registry/apis/provisioning/repository/test_test.go index 4e7b726ea56..1dd09596617 100644 --- a/pkg/registry/apis/provisioning/repository/test_test.go +++ b/pkg/registry/apis/provisioning/repository/test_test.go @@ -6,11 +6,12 @@ import ( "net/http" "testing" - provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" + + provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1" ) func TestValidateRepository(t *testing.T) { @@ -192,7 +193,7 @@ func TestTestRepository(t *testing.T) { name string repository *MockRepository expectedCode int - expectedErrs []string + expectedErrs []provisioning.ErrorDetails expectedError error }{ { @@ -208,7 +209,11 @@ func TestTestRepository(t *testing.T) { return m }(), expectedCode: http.StatusUnprocessableEntity, - expectedErrs: []string{"spec.title: Required value: a repository title must be given"}, + expectedErrs: []provisioning.ErrorDetails{{ + Type: metav1.CauseTypeFieldValueRequired, + Field: "spec.title", + Detail: "a repository title must be given", + }}, }, { name: "test passes", @@ -257,12 +262,18 @@ func TestTestRepository(t *testing.T) { m.On("Test", mock.Anything).Return(&provisioning.TestResults{ Code: http.StatusBadRequest, Success: false, - Errors: []string{"test failed"}, + Errors: []provisioning.ErrorDetails{{ + Type: metav1.CauseTypeFieldValueInvalid, + Field: "spec.property", + }}, }, nil) return m }(), expectedCode: http.StatusBadRequest, - expectedErrs: []string{"test failed"}, + expectedErrs: []provisioning.ErrorDetails{{ + Type: metav1.CauseTypeFieldValueInvalid, + Field: "spec.property", + }}, }, } diff --git a/pkg/registry/apis/provisioning/test.go b/pkg/registry/apis/provisioning/test.go index 85c8b3a2bf8..8197b00a96d 100644 --- a/pkg/registry/apis/provisioning/test.go +++ b/pkg/registry/apis/provisioning/test.go @@ -99,9 +99,9 @@ func (t *RepositoryTester) UpdateHealthStatus(ctx context.Context, cfg *provisio if res == nil { res = &provisioning.TestResults{ Success: false, - Errors: []string{ - "missing health status", - }, + Errors: []provisioning.ErrorDetails{{ + Detail: "missing health status", + }}, } } @@ -109,7 +109,11 @@ func (t *RepositoryTester) UpdateHealthStatus(ctx context.Context, cfg *provisio repo.Status.Health = provisioning.HealthStatus{ Healthy: res.Success, Checked: time.Now().UnixMilli(), - Message: res.Errors, + } + for _, err := range res.Errors { + if err.Detail != "" { + repo.Status.Health.Message = append(repo.Status.Health.Message, err.Detail) + } } _, err := t.client.Repositories(repo.GetNamespace()). diff --git a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json index 45dd20e4530..82b2537f563 100644 --- a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json +++ b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json @@ -2472,6 +2472,24 @@ } } }, + "com.github.grafana.grafana.pkg.apis.provisioning.v0alpha1.ErrorDetails": { + "type": "object", + "required": [ + "type" + ], + "properties": { + "detail": { + "type": "string" + }, + "field": { + "type": "string" + }, + "type": { + "type": "string", + "default": "" + } + } + }, "com.github.grafana.grafana.pkg.apis.provisioning.v0alpha1.ExportJobOptions": { "type": "object", "properties": { @@ -3762,20 +3780,16 @@ "format": "int32", "default": 0 }, - "details": { - "description": "Optional details", - "allOf": [ - { - "$ref": "#/components/schemas/com.github.grafana.grafana.pkg.apimachinery.apis.common.v0alpha1.Unstructured" - } - ] - }, "errors": { - "description": "Error descriptions", + "description": "Field related errors", "type": "array", "items": { - "type": "string", - "default": "" + "default": {}, + "allOf": [ + { + "$ref": "#/components/schemas/com.github.grafana.grafana.pkg.apis.provisioning.v0alpha1.ErrorDetails" + } + ] } }, "kind": { diff --git a/public/app/api/clients/provisioning/endpoints.gen.ts b/public/app/api/clients/provisioning/endpoints.gen.ts index 58fa48fb9f1..c665973eb50 100644 --- a/public/app/api/clients/provisioning/endpoints.gen.ts +++ b/public/app/api/clients/provisioning/endpoints.gen.ts @@ -1096,15 +1096,18 @@ export type ResourceList = { kind?: string; metadata?: ListMeta; }; +export type ErrorDetails = { + detail?: string; + field?: string; + type: string; +}; export type TestResults = { /** APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources */ apiVersion?: string; /** HTTP status code */ code: number; - /** Optional details */ - details?: Unstructured; - /** Error descriptions */ - errors?: string[]; + /** Field related errors */ + errors?: ErrorDetails[]; /** Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds */ kind?: string; /** Is the connection healthy */ diff --git a/public/app/api/clients/provisioning/index.ts b/public/app/api/clients/provisioning/index.ts index 3aeb73d552f..5695306ad6e 100644 --- a/public/app/api/clients/provisioning/index.ts +++ b/public/app/api/clients/provisioning/index.ts @@ -13,6 +13,7 @@ import { JobList, Repository, RepositoryList, + ErrorDetails, } from './endpoints.gen'; import { createOnCacheEntryAdded } from './utils/createOnCacheEntryAdded'; @@ -100,9 +101,11 @@ export const provisioningAPI = generatedAPI.enhanceEndpoints({ dispatch(notifyApp(createErrorNotification('Error validating repository', e))); } else if (typeof e === 'object' && 'error' in e && isFetchError(e.error)) { if (Array.isArray(e.error.data.errors) && e.error.data.errors.length) { - dispatch( - notifyApp(createErrorNotification('Error validating repository', e.error.data.errors.join('\n'))) - ); + const nonFieldErrors = e.error.data.errors.filter((err: ErrorDetails) => !err.field); + // Only show notification if there are errors that don't have a field, field errors are handled by the form + if (nonFieldErrors.length > 0) { + dispatch(notifyApp(createErrorNotification('Error validating repository'))); + } } } } diff --git a/public/app/features/provisioning/Job/ActiveJobStatus.tsx b/public/app/features/provisioning/Job/ActiveJobStatus.tsx deleted file mode 100644 index 37a9f4442f4..00000000000 --- a/public/app/features/provisioning/Job/ActiveJobStatus.tsx +++ /dev/null @@ -1,16 +0,0 @@ -import { Job } from 'app/api/clients/provisioning'; - -import { JobContent } from './JobContent'; -import { useJobStatusEffect } from './hooks'; - -export interface ActiveJobProps { - job: Job; - onStatusChange?: (success: boolean) => void; - onRunningChange?: (isRunning: boolean) => void; - onErrorChange?: (error: string | null) => void; -} - -export function ActiveJobStatus({ job, onStatusChange, onRunningChange, onErrorChange }: ActiveJobProps) { - useJobStatusEffect(job, onStatusChange, onRunningChange, onErrorChange); - return ; -} diff --git a/public/app/features/provisioning/Job/JobStatus.tsx b/public/app/features/provisioning/Job/JobStatus.tsx index 7ab68023055..632a82471af 100644 --- a/public/app/features/provisioning/Job/JobStatus.tsx +++ b/public/app/features/provisioning/Job/JobStatus.tsx @@ -4,8 +4,8 @@ import { Trans } from 'app/core/internationalization'; import { StepStatusInfo } from '../Wizard/types'; -import { ActiveJobStatus } from './ActiveJobStatus'; import { FinishedJobStatus } from './FinishedJobStatus'; +import { JobContent } from './JobContent'; export interface JobStatusProps { watch: Job; @@ -35,8 +35,13 @@ export function JobStatus({ watch, onStatusChange }: JobStatusProps) { ); } + if (activeQuery.isError) { + onStatusChange({ status: 'error', error: 'Error fetching active job' }); + return null; + } + if (activeJob) { - return ; + return ; } if (shouldCheckFinishedJobs) { diff --git a/public/app/features/provisioning/Job/hooks.ts b/public/app/features/provisioning/Job/hooks.ts deleted file mode 100644 index 04fe54b13f1..00000000000 --- a/public/app/features/provisioning/Job/hooks.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { useEffect } from 'react'; - -import { Job } from 'app/api/clients/provisioning'; -import { t } from 'app/core/internationalization'; - -// Shared hook for status change effects -export function useJobStatusEffect( - job?: Job, - onStatusChange?: (success: boolean) => void, - onRunningChange?: (isRunning: boolean) => void, - onErrorChange?: (error: string | null) => void -) { - useEffect(() => { - if (!job) { - return; - } - - if (onStatusChange && job.status?.state === 'success') { - onStatusChange(true); - if (onRunningChange) { - onRunningChange(false); - } - } - if (onErrorChange && job.status?.state === 'error') { - onErrorChange(job.status.message ?? t('provisioning.job-status.error-unknown', 'An unknown error occurred')); - if (onRunningChange) { - onRunningChange(false); - } - } - }, [job, onStatusChange, onErrorChange, onRunningChange]); -} diff --git a/public/app/features/provisioning/Wizard/ProvisioningWizard.tsx b/public/app/features/provisioning/Wizard/ProvisioningWizard.tsx index bdc91f790f1..cbcda614c32 100644 --- a/public/app/features/provisioning/Wizard/ProvisioningWizard.tsx +++ b/public/app/features/provisioning/Wizard/ProvisioningWizard.tsx @@ -4,7 +4,7 @@ import { FormProvider, useForm } from 'react-hook-form'; import { useNavigate } from 'react-router-dom-v5-compat'; import { AppEvents, GrafanaTheme2 } from '@grafana/data'; -import { getAppEvents } from '@grafana/runtime'; +import { getAppEvents, isFetchError } from '@grafana/runtime'; import { Alert, Box, Button, Stack, Text, useStyles2 } from '@grafana/ui'; import { useDeleteRepositoryMutation, useGetFrontendSettingsQuery } from 'app/api/clients/provisioning'; import { FormPrompt } from 'app/core/components/FormPrompt/FormPrompt'; @@ -14,11 +14,11 @@ import { getDefaultValues } from '../Config/defaults'; import { PROVISIONING_URL } from '../constants'; import { useCreateOrUpdateRepository } from '../hooks/useCreateOrUpdateRepository'; import { dataToSpec } from '../utils/data'; +import { getFormErrors } from '../utils/getFormErrors'; import { BootstrapStep } from './BootstrapStep'; import { ConnectStep } from './ConnectStep'; import { FinishStep } from './FinishStep'; -import { RequestErrorAlert } from './RequestErrorAlert'; import { Step, Stepper } from './Stepper'; import { SynchronizeStep } from './SynchronizeStep'; import { RepoType, StepStatusInfo, WizardFormData, WizardStep } from './types'; @@ -83,11 +83,12 @@ export function ProvisioningWizard({ type }: { type: RepoType }) { setValue, getValues, trigger, + setError, formState: { isDirty }, } = methods; const repoName = watch('repositoryName'); - const [submitData, saveRequest] = useCreateOrUpdateRepository(repoName); + const [submitData] = useCreateOrUpdateRepository(repoName); const [deleteRepository] = useDeleteRepositoryMutation(); const currentStepIndex = steps.findIndex((s) => s.id === activeStep); @@ -191,10 +192,17 @@ export function ProvisioningWizard({ type }: { type: RepoType }) { console.error('Saved repository without a name:', rsp); } } catch (error) { - setStepStatusInfo({ - status: 'error', - error: 'Repository connection failed', - }); + if (isFetchError(error)) { + const [field, errorMessage] = getFormErrors(error.data.errors); + if (field && errorMessage) { + setError(field, errorMessage); + } + } else { + setStepStatusInfo({ + status: 'error', + error: 'Repository connection failed', + }); + } } finally { setIsSubmitting(false); } @@ -210,7 +218,12 @@ export function ProvisioningWizard({ type }: { type: RepoType }) { if (activeStep === 'synchronize') { return stepStatusInfo.status !== 'success'; } - return isSubmitting || isCancelling || stepStatusInfo.status === 'running' || stepStatusInfo.status === 'error'; + return ( + isSubmitting || + isCancelling || + stepStatusInfo.status === 'running' || + (activeStep !== 'connection' && stepStatusInfo.status === 'error') + ); }; return ( @@ -228,13 +241,9 @@ export function ProvisioningWizard({ type }: { type: RepoType }) { - + {stepStatusInfo.status === 'error' && ( + + )}
{activeStep === 'connection' && } @@ -252,16 +261,8 @@ export function ProvisioningWizard({ type }: { type: RepoType }) { {activeStep === 'finish' && }
- {stepStatusInfo.status === 'error' && ( - - )} - -