From b2895ff71ea50212acd1e1ba45996eb6a3d1a78c Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Wed, 25 Sep 2024 16:36:19 +0300 Subject: [PATCH 1/4] feat: add ability to edit/import gitconfig when no configmap exists Signed-off-by: Oleksii Kurinnyi --- .../gitConfigApi/__tests__/index.spec.ts | 379 +++++++++++------- .../services/gitConfigApi/index.ts | 45 ++- .../services/helpers/prepareCoreV1API.ts | 3 + .../__snapshots__/index.spec.tsx.snap | 22 +- .../Email/__tests__/index.spec.tsx | 38 +- .../Form/SectionUser/Email/index.tsx | 70 ++-- .../SectionUser/Name/__tests__/index.spec.tsx | 35 +- .../GitConfig/Form/SectionUser/Name/index.tsx | 62 +-- 8 files changed, 396 insertions(+), 258 deletions(-) diff --git a/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts b/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts index 3ddd010a8..98e5a2f56 100644 --- a/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts +++ b/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts @@ -13,6 +13,7 @@ import { api } from '@eclipse-che/common'; import * as mockClient from '@kubernetes/client-node'; import { CoreV1Api, V1ConfigMap } from '@kubernetes/client-node'; +import { exec } from 'child_process'; import { IncomingMessage } from 'http'; import { GitConfigApiService } from '..'; @@ -29,198 +30,266 @@ const responseBody = { describe('Gitconfig API', () => { let gitConfigApiService: GitConfigApiService; - const stubCoreV1Api = { - readNamespacedConfigMap: () => { - return Promise.resolve({ - body: responseBody as V1ConfigMap, - response: {} as IncomingMessage, - }); - }, - patchNamespacedConfigMap: () => { - return Promise.resolve({ - body: responseBody as V1ConfigMap, - response: {} as IncomingMessage, - }); - }, - } as unknown as CoreV1Api; - const spyReadNamespacedConfigMap = jest.spyOn(stubCoreV1Api, 'readNamespacedConfigMap'); - const spyPatchNamespacedConfigMap = jest.spyOn(stubCoreV1Api, 'patchNamespacedConfigMap'); + describe('configmap not found', () => { + const stubCoreV1Api = { + createNamespacedConfigMap: () => { + return Promise.resolve({ + body: responseBody as V1ConfigMap, + response: {} as IncomingMessage, + }); + }, + readNamespacedConfigMap: () => { + return Promise.reject({ + body: {}, + response: {} as IncomingMessage, + statusCode: 404, + name: 'HttpError', + message: 'Not Found', + }); + }, + } as unknown as CoreV1Api; + const spyCreateNamespacedConfigMap = jest.spyOn(stubCoreV1Api, 'createNamespacedConfigMap'); + + beforeEach(() => { + const { KubeConfig } = mockClient; + const kubeConfig = new KubeConfig(); + kubeConfig.makeApiClient = jest.fn().mockImplementation(() => stubCoreV1Api); + + gitConfigApiService = new GitConfigApiService(kubeConfig); + }); - beforeEach(() => { - const { KubeConfig } = mockClient; - const kubeConfig = new KubeConfig(); - kubeConfig.makeApiClient = jest.fn().mockImplementation(() => stubCoreV1Api); + afterEach(() => { + jest.clearAllMocks(); + }); - gitConfigApiService = new GitConfigApiService(kubeConfig); - }); + it('should create the configmap and return gitconfig', async () => { + const resp = await gitConfigApiService.read(namespace); + + expect(resp.gitconfig).toStrictEqual({ + user: { + name: 'User One', + email: 'user-1@che', + }, + }); - afterEach(() => { - jest.clearAllMocks(); + expect(spyCreateNamespacedConfigMap).toHaveBeenCalledTimes(1); + expect(spyCreateNamespacedConfigMap).toHaveBeenCalledWith(namespace, { + data: { + gitconfig: `[user] +name="" +email="" +`, + }, + metadata: { + annotations: { + 'controller.devfile.io/mount-as': 'subpath', + 'controller.devfile.io/mount-path': '/etc/', + }, + labels: { + 'controller.devfile.io/mount-to-devworkspace': 'true', + 'controller.devfile.io/watch-configmap': 'true', + }, + name: 'workspace-userdata-gitconfig-configmap', + namespace, + }, + }); + }); }); - describe('reading gitconfig', () => { - it('should return gitconfig', async () => { - const resp = await gitConfigApiService.read(namespace); + describe('configmap found', () => { + const stubCoreV1Api = { + readNamespacedConfigMap: () => { + return Promise.resolve({ + body: responseBody as V1ConfigMap, + response: {} as IncomingMessage, + }); + }, + patchNamespacedConfigMap: () => { + return Promise.resolve({ + body: responseBody as V1ConfigMap, + response: {} as IncomingMessage, + }); + }, + } as unknown as CoreV1Api; + const spyReadNamespacedConfigMap = jest.spyOn(stubCoreV1Api, 'readNamespacedConfigMap'); + const spyPatchNamespacedConfigMap = jest.spyOn(stubCoreV1Api, 'patchNamespacedConfigMap'); + + beforeEach(() => { + const { KubeConfig } = mockClient; + const kubeConfig = new KubeConfig(); + kubeConfig.makeApiClient = jest.fn().mockImplementation(() => stubCoreV1Api); + + gitConfigApiService = new GitConfigApiService(kubeConfig); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe('reading gitconfig', () => { + it('should return gitconfig', async () => { + const resp = await gitConfigApiService.read(namespace); - expect(resp.gitconfig).toStrictEqual( - expect.objectContaining({ - user: expect.objectContaining({ - name: 'User One', - email: 'user-1@che', + expect(resp.gitconfig).toStrictEqual( + expect.objectContaining({ + user: expect.objectContaining({ + name: 'User One', + email: 'user-1@che', + }), }), - }), - ); + ); - expect(spyReadNamespacedConfigMap).toHaveBeenCalledTimes(1); - expect(spyPatchNamespacedConfigMap).not.toHaveBeenCalled(); - }); + expect(spyReadNamespacedConfigMap).toHaveBeenCalledTimes(1); + expect(spyPatchNamespacedConfigMap).not.toHaveBeenCalled(); + }); - it('should throw', async () => { - spyReadNamespacedConfigMap.mockRejectedValueOnce('404 not found'); + it('should throw', async () => { + spyReadNamespacedConfigMap.mockRejectedValueOnce('404 not found'); - try { - await gitConfigApiService.read(namespace); + try { + await gitConfigApiService.read(namespace); - // should not reach here - expect(false).toBeTruthy(); - } catch (e) { - expect((e as Error).message).toBe( - 'Unable to read gitconfig in the namespace "user-che": 404 not found', - ); - } + // should not reach here + expect(false).toBeTruthy(); + } catch (e) { + expect((e as Error).message).toBe( + 'Unable to read gitconfig in the namespace "user-che": 404 not found', + ); + } - expect(spyReadNamespacedConfigMap).toHaveBeenCalledTimes(1); - expect(spyPatchNamespacedConfigMap).not.toHaveBeenCalled(); + expect(spyReadNamespacedConfigMap).toHaveBeenCalledTimes(1); + expect(spyPatchNamespacedConfigMap).not.toHaveBeenCalled(); + }); }); - }); - describe('patching gitconfig', () => { - it('should patch and return gitconfig', async () => { - const newGitConfig = { - gitconfig: { - user: { - email: 'user-2@che', - name: 'User Two', + describe('patching gitconfig', () => { + it('should patch and return gitconfig', async () => { + const newGitConfig = { + gitconfig: { + user: { + email: 'user-2@che', + name: 'User Two', + }, }, - }, - } as api.IGitConfig; - await gitConfigApiService.patch(namespace, newGitConfig); - - expect(spyReadNamespacedConfigMap).toHaveBeenCalledTimes(1); - expect(spyPatchNamespacedConfigMap).toHaveBeenCalledTimes(1); - expect(spyPatchNamespacedConfigMap).toHaveBeenCalledWith( - 'workspace-userdata-gitconfig-configmap', - 'user-che', - { - data: { - gitconfig: `[user] + } as api.IGitConfig; + await gitConfigApiService.patch(namespace, newGitConfig); + + expect(spyReadNamespacedConfigMap).toHaveBeenCalledTimes(1); + expect(spyPatchNamespacedConfigMap).toHaveBeenCalledTimes(1); + expect(spyPatchNamespacedConfigMap).toHaveBeenCalledWith( + 'workspace-userdata-gitconfig-configmap', + 'user-che', + { + data: { + gitconfig: `[user] email="user-2@che" name="User Two" `, + }, }, - }, - undefined, - undefined, - undefined, - undefined, - undefined, - { headers: { 'content-type': 'application/strategic-merge-patch+json' } }, - ); - }); + undefined, + undefined, + undefined, + undefined, + undefined, + { headers: { 'content-type': 'application/strategic-merge-patch+json' } }, + ); + }); - it('should throw when can`t read the ConfigMap', async () => { - spyReadNamespacedConfigMap.mockRejectedValueOnce('404 not found'); + it('should throw when can`t read the ConfigMap', async () => { + spyReadNamespacedConfigMap.mockRejectedValueOnce('404 not found'); - const newGitConfig = { - gitconfig: { - user: { - email: 'user-2@che', - name: 'User Two', + const newGitConfig = { + gitconfig: { + user: { + email: 'user-2@che', + name: 'User Two', + }, }, - }, - } as api.IGitConfig; + } as api.IGitConfig; - try { - await gitConfigApiService.patch(namespace, newGitConfig); + try { + await gitConfigApiService.patch(namespace, newGitConfig); - // should not reach here - expect(false).toBeTruthy(); - } catch (e) { - expect((e as Error).message).toBe( - 'Unable to update gitconfig in the namespace "user-che".', - ); - } + // should not reach here + expect(false).toBeTruthy(); + } catch (e) { + expect((e as Error).message).toBe( + 'Unable to update gitconfig in the namespace "user-che".', + ); + } - expect(spyReadNamespacedConfigMap).toHaveBeenCalledTimes(1); - expect(spyPatchNamespacedConfigMap).toHaveBeenCalledTimes(0); - }); + expect(spyReadNamespacedConfigMap).toHaveBeenCalledTimes(1); + expect(spyPatchNamespacedConfigMap).toHaveBeenCalledTimes(0); + }); - it('should throw when failed to patch the ConfigMap', async () => { - spyPatchNamespacedConfigMap.mockRejectedValueOnce('some error'); + it('should throw when failed to patch the ConfigMap', async () => { + spyPatchNamespacedConfigMap.mockRejectedValueOnce('some error'); - const newGitConfig = { - gitconfig: { - user: { - email: 'user-2@che', - name: 'User Two', + const newGitConfig = { + gitconfig: { + user: { + email: 'user-2@che', + name: 'User Two', + }, }, - }, - } as api.IGitConfig; + } as api.IGitConfig; - try { - await gitConfigApiService.patch(namespace, newGitConfig); + try { + await gitConfigApiService.patch(namespace, newGitConfig); - // should not reach here - expect(false).toBeTruthy(); - } catch (e) { - expect((e as Error).message).toBe( - 'Unable to update gitconfig in the namespace "user-che": some error', - ); - } + // should not reach here + expect(false).toBeTruthy(); + } catch (e) { + expect((e as Error).message).toBe( + 'Unable to update gitconfig in the namespace "user-che": some error', + ); + } - expect(spyReadNamespacedConfigMap).toHaveBeenCalledTimes(1); - expect(spyPatchNamespacedConfigMap).toHaveBeenCalledTimes(1); - }); + expect(spyReadNamespacedConfigMap).toHaveBeenCalledTimes(1); + expect(spyPatchNamespacedConfigMap).toHaveBeenCalledTimes(1); + }); - it('should throw when conflict detected', async () => { - spyReadNamespacedConfigMap.mockResolvedValueOnce({ - body: { - metadata: { - resourceVersion: '2', - }, - data: { - gitconfig: `[user] + it('should throw when conflict detected', async () => { + spyReadNamespacedConfigMap.mockResolvedValueOnce({ + body: { + metadata: { + resourceVersion: '2', + }, + data: { + gitconfig: `[user] name="User Two" email="user-2@che" `, + }, + } as V1ConfigMap, + response: {} as IncomingMessage, + }); + + const newGitConfig = { + gitconfig: { + user: { + email: 'user-2@che', + name: 'User Two', + }, }, - } as V1ConfigMap, - response: {} as IncomingMessage, + resourceVersion: '1', + } as api.IGitConfig; + + try { + await gitConfigApiService.patch(namespace, newGitConfig); + + // should not reach here + expect(false).toBeTruthy(); + } catch (e) { + expect((e as Error).message).toBe( + 'Conflict detected. The gitconfig was modified in the namespace "user-che".', + ); + } + + expect(spyReadNamespacedConfigMap).toHaveBeenCalledTimes(1); + expect(spyPatchNamespacedConfigMap).toHaveBeenCalledTimes(0); }); - - const newGitConfig = { - gitconfig: { - user: { - email: 'user-2@che', - name: 'User Two', - }, - }, - resourceVersion: '1', - } as api.IGitConfig; - - try { - await gitConfigApiService.patch(namespace, newGitConfig); - - // should not reach here - expect(false).toBeTruthy(); - } catch (e) { - expect((e as Error).message).toBe( - 'Conflict detected. The gitconfig was modified in the namespace "user-che".', - ); - } - - expect(spyReadNamespacedConfigMap).toHaveBeenCalledTimes(1); - expect(spyPatchNamespacedConfigMap).toHaveBeenCalledTimes(0); }); }); }); diff --git a/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/index.ts b/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/index.ts index 1a4118e22..254e41e87 100644 --- a/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/index.ts +++ b/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/index.ts @@ -10,7 +10,7 @@ * Red Hat, Inc. - initial API and implementation */ -import { api } from '@eclipse-che/common'; +import { api, helpers } from '@eclipse-che/common'; import * as k8s from '@kubernetes/client-node'; import * as ini from 'multi-ini'; @@ -41,11 +41,54 @@ export class GitConfigApiService implements IGitConfigApi { return this.toGitConfig(response.body); } catch (error) { + if (helpers.errors.isKubeClientError(error) && error.statusCode === 404) { + // Create gitconfig configmap if it does not exist + return this.createGitConfigMap(namespace); + } + const message = `Unable to read gitconfig in the namespace "${namespace}"`; throw createError(error, GITCONFIG_API_ERROR_LABEL, message); } } + /** + * @throws + * Creates `gitconfig` ConfigMap in the given `namespace`. + */ + private async createGitConfigMap(namespace: string): Promise { + const configMap = new k8s.V1ConfigMap(); + configMap.metadata = { + name: GITCONFIG_CONFIGMAP, + namespace, + labels: { + 'controller.devfile.io/mount-to-devworkspace': 'true', + 'controller.devfile.io/watch-configmap': 'true', + }, + annotations: { + 'controller.devfile.io/mount-as': 'subpath', + 'controller.devfile.io/mount-path': '/etc/', + }, + }; + configMap.data = { + gitconfig: this.fromGitConfig({ + gitconfig: { + user: { + name: '', + email: '', + }, + }, + }), + }; + + try { + const response = await this.coreV1API.createNamespacedConfigMap(namespace, configMap); + return this.toGitConfig(response.body); + } catch (error) { + const message = `Unable to create gitconfig in the namespace "${namespace}"`; + throw createError(error, GITCONFIG_API_ERROR_LABEL, message); + } + } + /** * @throws * Updates `gitconfig` in the given `namespace` with `changedGitConfig`. diff --git a/packages/dashboard-backend/src/devworkspaceClient/services/helpers/prepareCoreV1API.ts b/packages/dashboard-backend/src/devworkspaceClient/services/helpers/prepareCoreV1API.ts index 245672835..9a30e9405 100644 --- a/packages/dashboard-backend/src/devworkspaceClient/services/helpers/prepareCoreV1API.ts +++ b/packages/dashboard-backend/src/devworkspaceClient/services/helpers/prepareCoreV1API.ts @@ -16,6 +16,7 @@ import { retryableExec } from '@/devworkspaceClient/services/helpers/retryableEx export type CoreV1API = Pick< k8s.CoreV1Api, + | 'createNamespacedConfigMap' | 'createNamespacedSecret' | 'listNamespace' | 'listNamespacedEvent' @@ -33,6 +34,8 @@ export type CoreV1API = Pick< export function prepareCoreV1API(kc: k8s.KubeConfig): CoreV1API { const coreV1API = kc.makeApiClient(k8s.CoreV1Api); return { + createNamespacedConfigMap: (...args: Parameters) => + retryableExec(() => coreV1API.createNamespacedConfigMap(...args)), createNamespacedSecret: (...args: Parameters) => retryableExec(() => coreV1API.createNamespacedSecret(...args)), listNamespace: (...args: Parameters) => diff --git a/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Email/__tests__/__snapshots__/index.spec.tsx.snap b/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Email/__tests__/__snapshots__/index.spec.tsx.snap index e909a3c32..ca40c0247 100644 --- a/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Email/__tests__/__snapshots__/index.spec.tsx.snap +++ b/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Email/__tests__/__snapshots__/index.spec.tsx.snap @@ -31,7 +31,7 @@ exports[`GitConfigUserEmail snapshot 1`] = ` >
- error + default
-
- The value is not a valid email address. -
`; @@ -106,7 +99,7 @@ exports[`GitConfigUserEmail snapshot, not loading 1`] = ` >
- error + default
-
- The value is not a valid email address. -
`; diff --git a/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Email/__tests__/index.spec.tsx b/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Email/__tests__/index.spec.tsx index b363b89e7..a253f87db 100644 --- a/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Email/__tests__/index.spec.tsx +++ b/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Email/__tests__/index.spec.tsx @@ -11,12 +11,15 @@ */ import { ValidatedOptions } from '@patternfly/react-core'; +import { StateMock } from '@react-mock/state'; import userEvent from '@testing-library/user-event'; import * as React from 'react'; -import getComponentRenderer, { screen } from '@/services/__mocks__/getComponentRenderer'; - -import { GitConfigUserEmail } from '..'; +import { + GitConfigUserEmail, + State, +} from '@/pages/UserPreferences/GitConfig/Form/SectionUser/Email'; +import getComponentRenderer, { screen, waitFor } from '@/services/__mocks__/getComponentRenderer'; jest.mock('@/components/InputGroupExtended'); @@ -67,14 +70,18 @@ describe('GitConfigUserEmail', () => { expect(screen.getByTestId('validated')).toHaveTextContent(ValidatedOptions.error); }); - it('should re-validate on component update', () => { - const { reRenderComponent } = renderComponent('', false); - - expect(screen.getByTestId('validated')).toHaveTextContent(ValidatedOptions.error); + it('should reset validation', async () => { + const localState: Partial = { + value: '', + validated: ValidatedOptions.error, + }; + const { reRenderComponent } = renderComponent('user@che.org', true, localState); - reRenderComponent('user@che.com', false); + reRenderComponent('user@che.org', false, localState); - expect(screen.getByTestId('validated')).toHaveTextContent(ValidatedOptions.default); + await waitFor(() => + expect(screen.getByTestId('validated')).toHaveTextContent(ValidatedOptions.default), + ); }); it('should handle value changing', async () => { @@ -88,6 +95,17 @@ describe('GitConfigUserEmail', () => { }); }); -function getComponent(value: string, isLoading: boolean): React.ReactElement { +function getComponent( + value: string, + isLoading: boolean, + localState?: Partial, +): React.ReactElement { + if (localState) { + return ( + + + + ); + } return ; } diff --git a/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Email/index.tsx b/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Email/index.tsx index 4e220fd71..0599169f2 100644 --- a/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Email/index.tsx +++ b/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Email/index.tsx @@ -28,8 +28,8 @@ export type Props = { onChange: (value: string, isValid: boolean) => void; }; export type State = { - errorMessage?: string; validated: ValidatedOptions | undefined; + value: string | undefined; }; export class GitConfigUserEmail extends React.PureComponent { @@ -37,59 +37,59 @@ export class GitConfigUserEmail extends React.PureComponent { super(props); this.state = { - validated: undefined, + validated: ValidatedOptions.default, + value: props.value, }; } - public componentDidMount(): void { - const { value } = this.props; - this.validate(value, true); - } - - public componentDidUpdate(prevProps: Readonly): void { - const { value } = this.props; - if (value !== prevProps.value) { - this.validate(value, true); + public componentDidUpdate(_prevProps: Readonly, prevState: Readonly): void { + if (prevState.value === this.state.value && this.props.value !== this.state.value) { + // reset the initial value + this.setState({ + value: this.props.value, + validated: ValidatedOptions.default, + }); } } private handleChange(value: string): void { - const isValid = this.validate(value); + const validate = this.validate(value); + const isValid = validate === ValidatedOptions.success; + + this.setState({ + value, + validated: validate, + }); this.props.onChange(value, isValid); } - private validate(value: string, initial = false): boolean { + private validate(value: string): ValidatedOptions { if (value.length === 0) { - this.setState({ - errorMessage: ERROR_REQUIRED_VALUE, - validated: ValidatedOptions.error, - }); - return false; + return ValidatedOptions.error; } if (value.length > MAX_LENGTH) { - this.setState({ - errorMessage: ERROR_MAX_LENGTH, - validated: ValidatedOptions.error, - }); - return false; + return ValidatedOptions.error; } if (!REGEX.test(value)) { - this.setState({ - errorMessage: ERROR_INVALID_EMAIL, - validated: ValidatedOptions.error, - }); - return false; + return ValidatedOptions.error; } - this.setState({ - errorMessage: undefined, - validated: initial === true ? ValidatedOptions.default : ValidatedOptions.success, - }); - return true; + return ValidatedOptions.success; } public render(): React.ReactElement { - const { isLoading, value } = this.props; - const { errorMessage, validated } = this.state; + const { isLoading } = this.props; + const { value = '', validated } = this.state; + + let errorMessage: string; + if (value.length === 0) { + errorMessage = ERROR_REQUIRED_VALUE; + } else if (value.length > MAX_LENGTH) { + errorMessage = ERROR_MAX_LENGTH; + } else if (!REGEX.test(value)) { + errorMessage = ERROR_INVALID_EMAIL; + } else { + errorMessage = ''; + } const fieldId = 'gitconfig-user-email'; diff --git a/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Name/__tests__/index.spec.tsx b/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Name/__tests__/index.spec.tsx index 869f3101f..437f37689 100644 --- a/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Name/__tests__/index.spec.tsx +++ b/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Name/__tests__/index.spec.tsx @@ -11,12 +11,12 @@ */ import { ValidatedOptions } from '@patternfly/react-core'; +import { StateMock } from '@react-mock/state'; import userEvent from '@testing-library/user-event'; import * as React from 'react'; -import getComponentRenderer, { screen } from '@/services/__mocks__/getComponentRenderer'; - -import { GitConfigUserName } from '..'; +import { GitConfigUserName, State } from '@/pages/UserPreferences/GitConfig/Form/SectionUser/Name'; +import getComponentRenderer, { screen, waitFor } from '@/services/__mocks__/getComponentRenderer'; jest.mock('@/components/InputGroupExtended'); @@ -58,14 +58,20 @@ describe('GitConfigUserName', () => { expect(screen.getByTestId('validated')).toHaveTextContent(ValidatedOptions.error); }); - it('should re-validate on component update', () => { - const { reRenderComponent } = renderComponent('', false); + it('should reset validation', async () => { + const initialValue = 'user name'; + const localState: Partial = { + value: '', + validated: ValidatedOptions.error, + }; - expect(screen.getByTestId('validated')).toHaveTextContent(ValidatedOptions.error); + const { reRenderComponent } = renderComponent(initialValue, true, localState); - reRenderComponent('valid name', false); + reRenderComponent(initialValue, false, localState); - expect(screen.getByTestId('validated')).toHaveTextContent(ValidatedOptions.default); + await waitFor(() => + expect(screen.getByTestId('validated')).toHaveTextContent(ValidatedOptions.default), + ); }); it('should handle value changing', async () => { @@ -79,6 +85,17 @@ describe('GitConfigUserName', () => { }); }); -function getComponent(value: string, isLoading: boolean): React.ReactElement { +function getComponent( + value: string, + isLoading: boolean, + localState?: Partial, +): React.ReactElement { + if (localState) { + return ( + + + + ); + } return ; } diff --git a/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Name/index.tsx b/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Name/index.tsx index 4d4cb1747..79384eba3 100644 --- a/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Name/index.tsx +++ b/packages/dashboard-frontend/src/pages/UserPreferences/GitConfig/Form/SectionUser/Name/index.tsx @@ -26,8 +26,8 @@ export type Props = { onChange: (value: string, isValid: boolean) => void; }; export type State = { - errorMessage?: string; validated: ValidatedOptions | undefined; + value: string | undefined; }; export class GitConfigUserName extends React.PureComponent { @@ -35,53 +35,55 @@ export class GitConfigUserName extends React.PureComponent { super(props); this.state = { - validated: undefined, + validated: ValidatedOptions.default, + value: props.value, }; } - public componentDidMount(): void { - const { value } = this.props; - this.validate(value, true); - } - - public componentDidUpdate(prevProps: Readonly): void { - const { value } = this.props; - if (value !== prevProps.value) { - this.validate(value, true); + public componentDidUpdate(_prevProps: Readonly, prevState: Readonly): void { + if (prevState.value === this.state.value && this.props.value !== this.state.value) { + // reset the initial value + this.setState({ + value: this.props.value, + validated: ValidatedOptions.default, + }); } } private handleChange(value: string): void { - const isValid = this.validate(value); + const validated = this.validate(value); + const isValid = validated === ValidatedOptions.success; + + this.setState({ + value, + validated, + }); this.props.onChange(value, isValid); } - private validate(value: string, initial = false): boolean { + private validate(value: string): ValidatedOptions { if (value.length === 0) { - this.setState({ - errorMessage: ERROR_REQUIRED_VALUE, - validated: ValidatedOptions.error, - }); - return false; + return ValidatedOptions.error; } if (value.length > MAX_LENGTH) { - this.setState({ - errorMessage: ERROR_MAX_LENGTH, - validated: ValidatedOptions.error, - }); - return false; + return ValidatedOptions.error; } - this.setState({ - errorMessage: undefined, - validated: initial === true ? ValidatedOptions.default : ValidatedOptions.success, - }); - return true; + return ValidatedOptions.success; } public render(): React.ReactElement { - const { isLoading, value } = this.props; - const { errorMessage, validated } = this.state; + const { isLoading } = this.props; + const { value = '', validated } = this.state; + + let errorMessage: string; + if (value.length === 0) { + errorMessage = ERROR_REQUIRED_VALUE; + } else if (value.length > MAX_LENGTH) { + errorMessage = ERROR_MAX_LENGTH; + } else { + errorMessage = ''; + } const fieldId = 'gitconfig-user-name'; From f25d35068540ce98579e8047b563bfd1d519a0c8 Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Thu, 26 Sep 2024 12:18:23 +0300 Subject: [PATCH 2/4] fixup! feat: add ability to edit/import gitconfig when no configmap exists --- .../services/gitConfigApi/__tests__/index.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts b/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts index 98e5a2f56..08ad15591 100644 --- a/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts +++ b/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts @@ -13,7 +13,6 @@ import { api } from '@eclipse-che/common'; import * as mockClient from '@kubernetes/client-node'; import { CoreV1Api, V1ConfigMap } from '@kubernetes/client-node'; -import { exec } from 'child_process'; import { IncomingMessage } from 'http'; import { GitConfigApiService } from '..'; From 5c946dc572c6b71e2bfaff3324888acc6fdee3f8 Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Thu, 26 Sep 2024 16:10:48 +0300 Subject: [PATCH 3/4] Update packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts Co-authored-by: Oleksii Orel --- .../gitConfigApi/__tests__/index.spec.ts | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts b/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts index 08ad15591..bb95a46f9 100644 --- a/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts +++ b/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts @@ -275,16 +275,20 @@ email="user-2@che" resourceVersion: '1', } as api.IGitConfig; - try { - await gitConfigApiService.patch(namespace, newGitConfig); + let isPatched = false; + let errorMessage = ''; - // should not reach here - expect(false).toBeTruthy(); - } catch (e) { - expect((e as Error).message).toBe( - 'Conflict detected. The gitconfig was modified in the namespace "user-che".', - ); - } + try { + await gitConfigApiService.patch(namespace, newGitConfig); + isPatched = true; + } catch (e: unknown) { + errorMessage = (e as Error).message; + } + + expect(isPatched).toBe(false); + expect(errorMessage).toBe( + 'Conflict detected. The gitconfig was modified in the namespace "user-che".', + ); expect(spyReadNamespacedConfigMap).toHaveBeenCalledTimes(1); expect(spyPatchNamespacedConfigMap).toHaveBeenCalledTimes(0); From 1af744acace75f371a720120e75543825a99228a Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Thu, 26 Sep 2024 17:26:41 +0300 Subject: [PATCH 4/4] fixup! Update packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts --- .../gitConfigApi/__tests__/index.spec.ts | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts b/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts index bb95a46f9..d7c44a92a 100644 --- a/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts +++ b/packages/dashboard-backend/src/devworkspaceClient/services/gitConfigApi/__tests__/index.spec.ts @@ -275,20 +275,20 @@ email="user-2@che" resourceVersion: '1', } as api.IGitConfig; - let isPatched = false; - let errorMessage = ''; + let isPatched = false; + let errorMessage = ''; - try { - await gitConfigApiService.patch(namespace, newGitConfig); - isPatched = true; - } catch (e: unknown) { - errorMessage = (e as Error).message; - } - - expect(isPatched).toBe(false); - expect(errorMessage).toBe( - 'Conflict detected. The gitconfig was modified in the namespace "user-che".', - ); + try { + await gitConfigApiService.patch(namespace, newGitConfig); + isPatched = true; + } catch (e: unknown) { + errorMessage = (e as Error).message; + } + + expect(isPatched).toBe(false); + expect(errorMessage).toBe( + 'Conflict detected. The gitconfig was modified in the namespace "user-che".', + ); expect(spyReadNamespacedConfigMap).toHaveBeenCalledTimes(1); expect(spyPatchNamespacedConfigMap).toHaveBeenCalledTimes(0);