From 8b9e57f2f6492116cfe401227101fd0429eb7d35 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Tue, 1 Jul 2025 09:17:36 -0700 Subject: [PATCH 1/5] OFREP: Enable with standard aggregation (#107349) --- pkg/registry/apis/ofrep/noop.go | 59 ++++++++ pkg/registry/apis/ofrep/register.go | 163 +++++++++++++++++++---- pkg/tests/apis/features/features_test.go | 53 ++++++++ 3 files changed, 250 insertions(+), 25 deletions(-) create mode 100644 pkg/registry/apis/ofrep/noop.go create mode 100644 pkg/tests/apis/features/features_test.go diff --git a/pkg/registry/apis/ofrep/noop.go b/pkg/registry/apis/ofrep/noop.go new file mode 100644 index 00000000000..69d0e7edd6d --- /dev/null +++ b/pkg/registry/apis/ofrep/noop.go @@ -0,0 +1,59 @@ +package ofrep + +import ( + "context" + "net/http" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apiserver/pkg/registry/rest" +) + +// This is a dummy connector that is not actually used for anything +// EXCEPT -- k8s requires *something* to be registered so it will add the storage to discovery. +// As a quick workaround, we register a noop storage; and then manually remove it from openapi +type NoopConnector struct{} + +var ( + _ rest.Connecter = (*NoopConnector)(nil) + _ rest.StorageMetadata = (*NoopConnector)(nil) + _ rest.Scoper = (*NoopConnector)(nil) + _ rest.SingularNameProvider = (*NoopConnector)(nil) +) + +func (r *NoopConnector) New() runtime.Object { + return &metav1.Status{} +} + +func (r *NoopConnector) NamespaceScoped() bool { + return true // namespaced +} + +func (r *NoopConnector) GetSingularName() string { + return "noop" +} + +func (r *NoopConnector) Destroy() { +} + +func (r *NoopConnector) ConnectMethods() []string { + return []string{"GET"} +} + +func (r *NoopConnector) NewConnectOptions() (runtime.Object, bool, string) { + return nil, false, "" +} + +func (r *NoopConnector) ProducesMIMETypes(verb string) []string { + return nil +} + +func (r *NoopConnector) ProducesObject(verb string) interface{} { + return r.New() +} + +func (r *NoopConnector) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) { + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + _, _ = w.Write([]byte("NOOP")) + }), nil +} diff --git a/pkg/registry/apis/ofrep/register.go b/pkg/registry/apis/ofrep/register.go index 344743780e3..b8f2126dbcb 100644 --- a/pkg/registry/apis/ofrep/register.go +++ b/pkg/registry/apis/ofrep/register.go @@ -7,23 +7,23 @@ import ( "io" "net/http" "net/url" - "strconv" - "strings" - "github.com/grafana/authlib/types" - "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/services/featuremgmt" - "github.com/grafana/grafana/pkg/setting" + "github.com/gorilla/mux" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/authorization/authorizer" + "k8s.io/apiserver/pkg/registry/rest" genericapiserver "k8s.io/apiserver/pkg/server" "k8s.io/kube-openapi/pkg/common" "k8s.io/kube-openapi/pkg/spec3" + "k8s.io/kube-openapi/pkg/validation/spec" - "github.com/gorilla/mux" + "github.com/grafana/authlib/types" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/apiserver/builder" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/setting" ) var _ builder.APIGroupBuilder = (*APIBuilder)(nil) @@ -32,6 +32,11 @@ var _ builder.APIGroupVersionProvider = (*APIBuilder)(nil) const ofrepPath = "/ofrep/v1/evaluate/flags" +var groupVersion = schema.GroupVersion{ + Group: "features.grafana.app", + Version: "v0alpha1", +} + type APIBuilder struct { providerType string url *url.URL @@ -66,18 +71,19 @@ func (b *APIBuilder) GetAuthorizer() authorizer.Authorizer { } func (b *APIBuilder) GetGroupVersion() schema.GroupVersion { - return schema.GroupVersion{ - Group: "features.grafana.app", - Version: "v0alpha1", - } + return groupVersion } func (b *APIBuilder) InstallSchema(scheme *runtime.Scheme) error { - metav1.AddToGroupVersion(scheme, b.GetGroupVersion()) - return scheme.SetVersionPriority(b.GetGroupVersion()) + metav1.AddToGroupVersion(scheme, groupVersion) + scheme.AddKnownTypes(groupVersion, &metav1.Status{}) // for noop + return scheme.SetVersionPriority(groupVersion) } func (b *APIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.APIGroupInfo, opts builder.APIGroupOptions) error { + storage := map[string]rest.Storage{} + storage["noop"] = &NoopConnector{} + apiGroupInfo.VersionedResourcesStorageMap[groupVersion.Version] = storage return nil } @@ -91,20 +97,125 @@ func (b *APIBuilder) AllowedV0Alpha1Resources() []string { return []string{builder.AllResourcesAllowed} } +func (b *APIBuilder) PostProcessOpenAPI(oas *spec3.OpenAPI) (*spec3.OpenAPI, error) { + oas.Info.Description = "Proxy access to open feature flags" + + // Remove the NOOP connector + delete(oas.Paths.Paths, "/apis/"+groupVersion.String()+"/namespaces/{namespace}/noop/{name}") + return oas, nil +} + func (b *APIBuilder) GetAPIRoutes(gv schema.GroupVersion) *builder.APIRoutes { + evaluationContext := &spec3.RequestBody{ + RequestBodyProps: spec3.RequestBodyProps{ + Description: "EvaluationContext provides ambient information for the purposes of flag evaluation", + Content: map[string]*spec3.MediaType{ + "application/json": { + MediaTypeProps: spec3.MediaTypeProps{ + Schema: spec.MapProperty(spec.MapProperty(nil)), + Example: map[string]map[string]any{ + "context": { + "targetingKey": "1234", + "grafana_version": "12.0.0", + }, + }, + }, + }, + }}} + return &builder.APIRoutes{ Namespace: []builder.APIRouteHandler{ { Path: "ofrep/v1/evaluate/flags/", Spec: &spec3.PathProps{ - Post: &spec3.Operation{}, + Post: &spec3.Operation{ + OperationProps: spec3.OperationProps{ + Tags: []string{"Evaluate"}, + Description: "Evaluate all flags", + Parameters: []*spec3.Parameter{ + { + ParameterProps: spec3.ParameterProps{ + Name: "namespace", + In: "path", + Required: true, + Example: "default", + Description: "workspace", + Schema: spec.StringProperty(), + }, + }, + }, + RequestBody: evaluationContext, + Responses: &spec3.Responses{ + ResponsesProps: spec3.ResponsesProps{ + StatusCodeResponses: map[int]*spec3.Response{ + 200: { + ResponseProps: spec3.ResponseProps{ + Content: map[string]*spec3.MediaType{ + "application/json": { + MediaTypeProps: spec3.MediaTypeProps{ + Schema: spec.MapProperty(nil), // TODO... real type? + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, }, Handler: b.allFlagsHandler, }, { Path: "ofrep/v1/evaluate/flags/{flagKey}", Spec: &spec3.PathProps{ - Post: &spec3.Operation{}, + Post: &spec3.Operation{ + OperationProps: spec3.OperationProps{ + Tags: []string{"Evaluate"}, + Description: "Evaluate a single flag", + Parameters: []*spec3.Parameter{ + { + ParameterProps: spec3.ParameterProps{ + Name: "namespace", + In: "path", + Required: true, + Example: "default", + Description: "workspace", + Schema: spec.StringProperty(), + }, + }, + { + ParameterProps: spec3.ParameterProps{ + Name: "flagKey", + In: "path", + Required: true, + Example: "testflag", + Description: "flag key", + Schema: spec.StringProperty(), + }, + }, + }, + RequestBody: evaluationContext, + Responses: &spec3.Responses{ + ResponsesProps: spec3.ResponsesProps{ + StatusCodeResponses: map[int]*spec3.Response{ + 200: { + ResponseProps: spec3.ResponseProps{ + Content: map[string]*spec3.MediaType{ + "application/json": { + MediaTypeProps: spec3.MediaTypeProps{ + Schema: spec.MapProperty(nil), // TODO, real type + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, }, Handler: b.oneFlagHandler, }, @@ -168,29 +279,25 @@ func writeResponse(statusCode int, result any, logger log.Logger, w http.Respons } } -func (b *APIBuilder) stackIdFromEvalCtx(body []byte) string { +func (b *APIBuilder) stackIdFromEvalCtx(body []byte) int64 { // Extract stackID from request body without consuming it var evalCtx struct { Context struct { - StackID int32 `json:"stackId"` + StackID int64 `json:"stackId"` // TODO -- replace with namespace "stackId" ONLY makes sense in cloud } `json:"context"` } if err := json.Unmarshal(body, &evalCtx); err != nil { b.logger.Debug("Failed to unmarshal evaluation context", "error", err, "body", string(body)) - return "" + return 0 } if evalCtx.Context.StackID <= 0 { b.logger.Debug("Invalid or missing stackId in evaluation context", "stackId", evalCtx.Context.StackID) - return "" + return 0 } - return strconv.Itoa(int(evalCtx.Context.StackID)) -} - -func removeStackPrefix(tenant string) string { - return strings.TrimPrefix(tenant, "stacks-") + return evalCtx.Context.StackID } // isAuthenticatedRequest returns true if the request is authenticated @@ -217,6 +324,12 @@ func (b *APIBuilder) validateNamespace(r *http.Request) bool { namespace = mux.Vars(r)["namespace"] } + info, err := types.ParseNamespace(namespace) + if err != nil { + b.logger.Error("Error parsing namespace", "error", err) + return false + } + // Extract stackId from feature flag evaluation context body, err := io.ReadAll(r.Body) if err != nil { @@ -226,7 +339,7 @@ func (b *APIBuilder) validateNamespace(r *http.Request) bool { r.Body = io.NopCloser(bytes.NewBuffer(body)) // "default" namespace case can only occur in on-prem grafana - if b.stackIdFromEvalCtx(body) == removeStackPrefix(namespace) || namespace == "default" { + if b.stackIdFromEvalCtx(body) == info.StackID { return true } diff --git a/pkg/tests/apis/features/features_test.go b/pkg/tests/apis/features/features_test.go new file mode 100644 index 00000000000..1a4b05a898d --- /dev/null +++ b/pkg/tests/apis/features/features_test.go @@ -0,0 +1,53 @@ +package features + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/tests/apis" + "github.com/grafana/grafana/pkg/tests/testinfra" + "github.com/grafana/grafana/pkg/tests/testsuite" +) + +func TestMain(m *testing.M) { + testsuite.Run(m) +} + +func TestIntegrationFeatures(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + // Enable a random flag -- check that it is reported as enabled + flag := featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs + helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{ + AppModeProduction: true, + DisableAnonymous: false, // allow anon user + EnableFeatureToggles: []string{ + flag, // used in test below + }, + }) + + t.Run("Test evaluate flags", func(t *testing.T) { + rsp := apis.DoRequest(helper, apis.RequestParams{ + Method: http.MethodPost, + Path: "/apis/features.grafana.app/v0alpha1/namespaces/default/ofrep/v1/evaluate/flags/" + flag, + User: helper.Org1.Admin, + }, &map[string]any{}) + + require.Equal(t, 200, rsp.Response.StatusCode) + require.JSONEq(t, `{ + "Value": true, + "FlagKey": "`+flag+`", + "FlagType": 0, + "Variant": "enabled", + "Reason": "STATIC", + "ErrorCode": "", + "ErrorMessage": "", + "FlagMetadata": {} + }`, string(rsp.Body)) + }) +} From fdf4935e423252b8ea55628a09f9cd16a43b1359 Mon Sep 17 00:00:00 2001 From: Yunwen Zheng Date: Tue, 1 Jul 2025 12:19:48 -0400 Subject: [PATCH 2/5] Git sync create folder flow refactor to use new hook (#107422) * Git sync create folder flow refactor to use new hook --------- Co-authored-by: Alex Khomenko --- .betterer.results | 6 - .../components/CreateNewButton.tsx | 6 +- .../DeleteProvisionedFolderForm.test.tsx | 4 +- .../DeleteProvisionedFolderForm.tsx | 4 +- .../NewProvisionedFolderForm.test.tsx | 157 +++++----- .../components/NewProvisionedFolderForm.tsx | 270 ++++++++---------- ... => ResourceEditFormSharedFields.test.tsx} | 8 +- ...s.tsx => ResourceEditFormSharedFields.tsx} | 27 +- .../SaveProvisionedDashboardForm.tsx | 4 +- .../DeleteProvisionedDashboardForm.test.tsx | 4 +- .../DeleteProvisionedDashboardForm.tsx | 4 +- public/locales/en-US/grafana.json | 6 - 12 files changed, 225 insertions(+), 275 deletions(-) rename public/app/features/dashboard-scene/components/Provisioned/{DashboardEditFormSharedFields.test.tsx => ResourceEditFormSharedFields.test.tsx} (96%) rename public/app/features/dashboard-scene/components/Provisioned/{DashboardEditFormSharedFields.tsx => ResourceEditFormSharedFields.tsx} (82%) diff --git a/.betterer.results b/.betterer.results index cc04defd879..f5c2127333f 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1493,12 +1493,6 @@ exports[`better eslint`] = { "public/app/features/browse-dashboards/components/NewFolderForm.tsx:5381": [ [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "0"] ], - "public/app/features/browse-dashboards/components/NewProvisionedFolderForm.tsx:5381": [ - [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "0"], - [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "1"], - [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "2"], - [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "3"] - ], "public/app/features/browse-dashboards/state/index.ts:5381": [ [0, 0, 0, "Do not use export all (\`export * from ...\`)", "0"], [0, 0, 0, "Do not use export all (\`export * from ...\`)", "1"], diff --git a/public/app/features/browse-dashboards/components/CreateNewButton.tsx b/public/app/features/browse-dashboards/components/CreateNewButton.tsx index 600d0038224..bb00a254d31 100644 --- a/public/app/features/browse-dashboards/components/CreateNewButton.tsx +++ b/public/app/features/browse-dashboards/components/CreateNewButton.tsx @@ -107,11 +107,7 @@ export default function CreateNewButton({ parentFolder, canCreateDashboard, canC size="sm" > {parentFolder?.managedBy === ManagerKind.Repo || isProvisionedInstance ? ( - setShowNewFolderDrawer(false)} - onCancel={() => setShowNewFolderDrawer(false)} - parentFolder={parentFolder} - /> + setShowNewFolderDrawer(false)} parentFolder={parentFolder} /> ) : ( setShowNewFolderDrawer(false)} /> )} diff --git a/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.test.tsx b/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.test.tsx index d6f62f10e0f..f56e50bf023 100644 --- a/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.test.tsx +++ b/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.test.tsx @@ -33,8 +33,8 @@ jest.mock('./BrowseActions/DescendantCount', () => ({ DescendantCount: () =>
2 folders, 5 dashboards
, })); -jest.mock('app/features/dashboard-scene/components/Provisioned/DashboardEditFormSharedFields', () => ({ - DashboardEditFormSharedFields: () =>
, +jest.mock('app/features/dashboard-scene/components/Provisioned/ResourceEditFormSharedFields', () => ({ + ResourceEditFormSharedFields: () =>
, })); const mockUseDeleteRepositoryFilesMutation = useDeleteRepositoryFilesWithPathMutation as jest.MockedFunction< diff --git a/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.tsx b/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.tsx index fac9bbc6864..995f68bbd23 100644 --- a/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.tsx +++ b/public/app/features/browse-dashboards/components/DeleteProvisionedFolderForm.tsx @@ -8,7 +8,7 @@ import { Box, Button, Stack } from '@grafana/ui'; import { Folder } from 'app/api/clients/folder/v1beta1'; import { RepositoryView, useDeleteRepositoryFilesWithPathMutation } from 'app/api/clients/provisioning/v0alpha1'; import { AnnoKeySourcePath } from 'app/features/apiserver/types'; -import { DashboardEditFormSharedFields } from 'app/features/dashboard-scene/components/Provisioned/DashboardEditFormSharedFields'; +import { ResourceEditFormSharedFields } from 'app/features/dashboard-scene/components/Provisioned/ResourceEditFormSharedFields'; import { BaseProvisionedFormData } from 'app/features/dashboard-scene/saving/shared'; import { FolderDTO } from 'app/types'; @@ -121,7 +121,7 @@ function FormContent({ /> - { }; }); -jest.mock('app/api/clients/folder/v1beta1', () => { +jest.mock('../hooks/useProvisionedFolderFormData', () => { return { - useGetFolderQuery: jest.fn(), + useProvisionedFolderFormData: jest.fn(), }; }); @@ -53,12 +52,6 @@ jest.mock('app/features/provisioning/hooks/usePullRequestParam', () => { }; }); -jest.mock('app/features/provisioning/hooks/useGetResourceRepositoryView', () => { - return { - useGetResourceRepositoryView: jest.fn(), - }; -}); - jest.mock('react-router-dom-v5-compat', () => { const actual = jest.requireActual('react-router-dom-v5-compat'); return { @@ -79,17 +72,15 @@ jest.mock('../../dashboard-scene/saving/provisioned/defaults', () => { }); interface Props { - onSubmit: () => void; - onCancel: () => void; - parentFolder: FolderDTO; + onDismiss?: () => void; + parentFolder?: FolderDTO; } -function setup(props: Partial = {}) { +function setup(props: Partial = {}, hookData = mockHookData) { const user = userEvent.setup(); const defaultProps: Props = { - onSubmit: jest.fn(), - onCancel: jest.fn(), + onDismiss: jest.fn(), parentFolder: { id: 1, uid: 'folder-uid', @@ -108,6 +99,8 @@ function setup(props: Partial = {}) { ...props, }; + (useProvisionedFolderFormData as jest.Mock).mockReturnValue(hookData); + return { user, ...render(), @@ -123,6 +116,40 @@ const mockRequest = { data: { resource: { upsert: { metadata: { name: 'new-folder' } } } }, }; +const mockHookData: ProvisionedFolderFormDataResult = { + repository: { + name: 'test-repo', + title: 'Test Repository', + type: 'github', + workflows: ['write', 'branch'], + target: 'folder', + }, + folder: { + metadata: { + annotations: { + 'grafana.app/sourcePath': '/dashboards', + }, + }, + spec: { + title: '', + }, + status: {}, + }, + workflowOptions: [ + { label: 'Commit directly', value: 'write' }, + { label: 'Create a branch', value: 'branch' }, + ], + isGitHub: true, + initialValues: { + title: '', + comment: '', + ref: 'folder/test-timestamp', + repo: 'test-repo', + path: '/dashboards', + workflow: 'write', + }, +}; + describe('NewProvisionedFolderForm', () => { beforeEach(() => { jest.clearAllMocks(); @@ -133,39 +160,11 @@ describe('NewProvisionedFolderForm', () => { }; (getAppEvents as jest.Mock).mockReturnValue(mockAppEvents); - (useGetResourceRepositoryView as jest.Mock).mockReturnValue({ - isLoading: false, - repository: { - name: 'test-repo', - title: 'Test Repository', - type: 'github', - github: { - url: 'https://github.com/grafana/grafana', - branch: 'main', - }, - workflows: [{ name: 'default', path: 'workflows/default.json' }], - }, - }); - - // Mock useGetFolderQuery - (useGetFolderQuery as jest.Mock).mockReturnValue({ - data: { - metadata: { - annotations: { - 'source.path': '/dashboards', - }, - }, - }, - isLoading: false, - isError: false, - }); - // Mock usePullRequestParam (usePullRequestParam as jest.Mock).mockReturnValue(null); // Mock useCreateRepositoryFilesWithPathMutation const mockCreate = jest.fn(); - (useCreateRepositoryFilesWithPathMutation as jest.Mock).mockReturnValue([mockCreate, mockRequest]); (validationSrv.validateNewFolderName as jest.Mock).mockResolvedValue(true); @@ -182,25 +181,27 @@ describe('NewProvisionedFolderForm', () => { expect(screen.getByRole('button', { name: /cancel/i })).toBeInTheDocument(); }); - it('should show loading state when repository data is loading', () => { - (useGetResourceRepositoryView as jest.Mock).mockReturnValue({ - isLoading: true, - }); - - setup(); - - expect(screen.getByTestId('Spinner')).toBeInTheDocument(); + it('should return null when initialValues is not available', () => { + const { container } = setup( + {}, + { + ...mockHookData, + initialValues: undefined, + } + ); + expect(container.firstChild).toBeNull(); }); it('should show error when repository is not found', () => { - (useGetResourceRepositoryView as jest.Mock).mockReturnValue({ - isLoading: false, - repository: undefined, - }); - - setup(); - - expect(screen.getByText('Repository not found')).toBeInTheDocument(); + const { container } = setup( + {}, + { + ...mockHookData, + repository: undefined, + initialValues: undefined, + } + ); + expect(container.firstChild).toBeNull(); }); it('should show branch field when branch workflow is selected', async () => { @@ -289,6 +290,7 @@ describe('NewProvisionedFolderForm', () => { expect.objectContaining({ ref: undefined, // write workflow uses undefined ref name: 'test-repo', + path: '/dashboards/new-test-folder/', message: 'Creating a new test folder', body: { title: 'New Test Folder', @@ -298,8 +300,8 @@ describe('NewProvisionedFolderForm', () => { ); }); - // Check if onSubmit was called - expect(props.onSubmit).toHaveBeenCalled(); + // Check if onDismiss was called + expect(props.onDismiss).toHaveBeenCalled(); }); it('should create folder with branch workflow', async () => { @@ -341,6 +343,7 @@ describe('NewProvisionedFolderForm', () => { expect.objectContaining({ ref: 'feature/new-folder', name: 'test-repo', + path: '/dashboards/branch-folder/', message: 'Create folder: Branch Folder', body: { title: 'Branch Folder', @@ -415,33 +418,31 @@ describe('NewProvisionedFolderForm', () => { expect(screen.getByRole('link')).toHaveTextContent('https://github.com/grafana/grafana/pull/1234'); }); - it('should call onCancel when cancel button is clicked', async () => { + it('should call onDismiss when cancel button is clicked', async () => { const { user, props } = setup(); // Click cancel button const cancelButton = screen.getByRole('button', { name: /cancel/i }); await user.click(cancelButton); - // Check if onCancel was called - expect(props.onCancel).toHaveBeenCalled(); + expect(props.onDismiss).toHaveBeenCalled(); }); it('should show read-only alert when repository has no workflows', () => { // Mock repository with empty workflows array - (useGetResourceRepositoryView as jest.Mock).mockReturnValue({ - repository: { - name: 'test-repo', - title: 'Test Repository', - type: 'github', - github: { - url: 'https://github.com/grafana/grafana', - branch: 'main', + setup( + {}, + { + ...mockHookData, + repository: { + name: 'test-repo', + title: 'Test Repository', + type: 'github', + workflows: [], + target: 'folder', }, - workflows: [], - }, - }); - - setup(); + } + ); // Read-only alert should be visible expect(screen.getByText('This repository is read only')).toBeInTheDocument(); diff --git a/public/app/features/browse-dashboards/components/NewProvisionedFolderForm.tsx b/public/app/features/browse-dashboards/components/NewProvisionedFolderForm.tsx index 617d96d5087..ed03a23cb7a 100644 --- a/public/app/features/browse-dashboards/components/NewProvisionedFolderForm.tsx +++ b/public/app/features/browse-dashboards/components/NewProvisionedFolderForm.tsx @@ -1,73 +1,52 @@ import { useEffect } from 'react'; -import { Controller, useForm } from 'react-hook-form'; +import { FormProvider, useForm } from 'react-hook-form'; import { useNavigate } from 'react-router-dom-v5-compat'; import { AppEvents } from '@grafana/data'; import { Trans, t } from '@grafana/i18n'; import { getAppEvents } from '@grafana/runtime'; -import { Alert, Button, Field, Input, RadioButtonGroup, Spinner, Stack, TextArea } from '@grafana/ui'; -import { useCreateRepositoryFilesWithPathMutation } from 'app/api/clients/provisioning/v0alpha1'; +import { Alert, Button, Field, Input, Stack } from '@grafana/ui'; +import { Folder } from 'app/api/clients/folder/v1beta1'; +import { RepositoryView, useCreateRepositoryFilesWithPathMutation } from 'app/api/clients/provisioning/v0alpha1'; import { AnnoKeySourcePath, Resource } from 'app/features/apiserver/types'; -import { getDefaultWorkflow, getWorkflowOptions } from 'app/features/dashboard-scene/saving/provisioned/defaults'; +import { ResourceEditFormSharedFields } from 'app/features/dashboard-scene/components/Provisioned/ResourceEditFormSharedFields'; +import { BaseProvisionedFormData } from 'app/features/dashboard-scene/saving/shared'; import { validationSrv } from 'app/features/manage-dashboards/services/ValidationSrv'; -import { BranchValidationError } from 'app/features/provisioning/Shared/BranchValidationError'; import { PROVISIONING_URL } from 'app/features/provisioning/constants'; -import { useGetResourceRepositoryView } from 'app/features/provisioning/hooks/useGetResourceRepositoryView'; import { usePullRequestParam } from 'app/features/provisioning/hooks/usePullRequestParam'; -import { WorkflowOption } from 'app/features/provisioning/types'; -import { validateBranchName } from 'app/features/provisioning/utils/git'; import { FolderDTO } from 'app/types'; -type FormData = { - ref?: string; - path: string; - comment?: string; - repo: string; - workflow?: WorkflowOption; - title: string; -}; - +import { useProvisionedFolderFormData } from '../hooks/useProvisionedFolderFormData'; +interface FormProps extends Props { + initialValues: BaseProvisionedFormData; + repository?: RepositoryView; + workflowOptions: Array<{ label: string; value: string }>; + folder?: Folder; + isGitHub: boolean; +} interface Props { - onSubmit: () => void; - onCancel: () => void; parentFolder?: FolderDTO; + onDismiss?: () => void; } -const initialFormValues: Partial = { - title: '', - comment: '', - ref: `folder/${Date.now()}`, -}; - -// TODO: use useProvisionedFolderFormData hook to manage form data and repository state -export function NewProvisionedFolderForm({ onSubmit, onCancel, parentFolder }: Props) { - const { repository, folder, isLoading } = useGetResourceRepositoryView({ folderName: parentFolder?.uid }); +function FormContent({ initialValues, repository, workflowOptions, folder, isGitHub, onDismiss }: FormProps) { const prURL = usePullRequestParam(); const navigate = useNavigate(); const [create, request] = useCreateRepositoryFilesWithPathMutation(); - const isGitHub = Boolean(repository?.type === 'github'); - - const { - register, - handleSubmit, - watch, - formState: { errors }, - control, - setValue, - } = useForm({ defaultValues: { ...initialFormValues, workflow: getDefaultWorkflow(repository) } }); + const methods = useForm({ + defaultValues: initialValues, + mode: 'onBlur', // Validates when user leaves the field + }); + const { handleSubmit, watch, register, formState } = methods; const [workflow, ref] = watch(['workflow', 'ref']); - useEffect(() => { - setValue('workflow', getDefaultWorkflow(repository)); - }, [repository, setValue]); - // TODO: replace with useProvisionedRequestHandler hook useEffect(() => { const appEvents = getAppEvents(); if (request.isSuccess && repository) { - onSubmit(); + onDismiss?.(); appEvents.publish({ type: AppEvents.alertSuccess.name, @@ -101,20 +80,7 @@ export function NewProvisionedFolderForm({ onSubmit, onCancel, parentFolder }: P ], }); } - }, [request.isSuccess, request.isError, request.error, onSubmit, ref, request.data, workflow, navigate, repository]); - - if (isLoading) { - return ; - } - - if (!repository) { - return ( - - ); - } + }, [request.isSuccess, request.isError, request.error, ref, request.data, workflow, navigate, repository, onDismiss]); const validateFolderName = async (folderName: string) => { try { @@ -128,7 +94,7 @@ export function NewProvisionedFolderForm({ onSubmit, onCancel, parentFolder }: P } }; - const doSave = async ({ ref, title, workflow, comment }: FormData) => { + const doSave = async ({ ref, title, workflow, comment }: BaseProvisionedFormData) => { const repoName = repository?.name; if (!title || !repoName) { return; @@ -163,107 +129,103 @@ export function NewProvisionedFolderForm({ onSubmit, onCancel, parentFolder }: P }; return ( -
- - {!repository?.workflows?.length && ( - + + + {!repository?.workflows?.length && ( + + + If you have direct access to the target, copy the JSON and paste it there. + + + )} + + - - If you have direct access to the target, copy the JSON and paste it there. - - - )} + + - - - - {/* TODO: use DashboardEditFormSharedFields to replace comment and workflow input*/} - -