From 08ced72b30a14dbb438ac2203f6c58968f71ba75 Mon Sep 17 00:00:00 2001 From: Celia Usero Navarro Date: Tue, 20 Aug 2024 13:31:52 +0100 Subject: [PATCH 1/4] fix: create branch or source files in remote before pushing --- .../src/storage/GitlabTokenStorage.ts | 29 +++++++++++++++++-- .../src/types/StorageType.ts | 1 + 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/packages/tokens-studio-for-figma/src/storage/GitlabTokenStorage.ts b/packages/tokens-studio-for-figma/src/storage/GitlabTokenStorage.ts index 8d5492a55..3e610b59a 100644 --- a/packages/tokens-studio-for-figma/src/storage/GitlabTokenStorage.ts +++ b/packages/tokens-studio-for-figma/src/storage/GitlabTokenStorage.ts @@ -30,11 +30,17 @@ export class GitlabTokenStorage extends GitTokenStorage { protected repoPathWithNamespace: string; + protected source?: string; + + protected previousSourceBranch?: string; + constructor( secret: string, repository: string, repoPathWithNamespace: string, baseUrl?: string, + branch = 'main', + previousSourceBranch = 'main', ) { super(secret, '', repository, baseUrl); @@ -43,6 +49,8 @@ export class GitlabTokenStorage extends GitTokenStorage { token: this.secret, host: this.baseUrl || undefined, }); + this.source = branch; + this.previousSourceBranch = previousSourceBranch; } public async assignProjectId() { @@ -204,6 +212,24 @@ export class GitlabTokenStorage extends GitTokenStorage { const rootPath = this.path.endsWith('.json') ? this.path.split('/').slice(0, -1).join('/') : this.path; + if (shouldCreateBranch && !branches.includes(branch)) { + const sourceBranch = this.previousSourceBranch || this.source; + await this.createBranch(branch, sourceBranch); + } + // Directories cannot be created empty (Source: https://gitlab.com/gitlab-org/gitlab/-/issues/247503) + const pathToCreate = this.path.endsWith('.json') ? this.path : `${this.path}/.gitkeep`; + try { + await this.gitlabClient.RepositoryFiles.show(this.projectId, pathToCreate, branch); + } catch (e) { + await this.gitlabClient.RepositoryFiles.create( + this.projectId, + pathToCreate, + branch, + '{}', + 'Initial commit', + ); + } + const tree = await this.gitlabClient.Repositories.allRepositoryTrees(this.projectId, { path: rootPath, ref: branch, @@ -235,9 +261,6 @@ export class GitlabTokenStorage extends GitTokenStorage { branch, message, gitlabActions, - shouldCreateBranch ? { - startBranch: branches[0], - } : undefined, ); return !!response; } catch (e: any) { diff --git a/packages/tokens-studio-for-figma/src/types/StorageType.ts b/packages/tokens-studio-for-figma/src/types/StorageType.ts index 1ebd4bf87..dc030a7f4 100644 --- a/packages/tokens-studio-for-figma/src/types/StorageType.ts +++ b/packages/tokens-studio-for-figma/src/types/StorageType.ts @@ -57,6 +57,7 @@ StorageProviderType.GITLAB, filePath: string; // this is the path to the token file or files (depends on multifile support) baseUrl?: string; // this is the base API url. This is important for self hosted environments commitDate?: Date; // this is the commit sha of the current file or folder + previousSourceBranch?: string; // optional: allows pushing changes to remote based on an existing branch } >; From 32fb9c5adc47a7058ed1de5bf3457c856c84e1f6 Mon Sep 17 00:00:00 2001 From: Celia Usero Navarro Date: Tue, 20 Aug 2024 13:32:36 +0100 Subject: [PATCH 2/4] fix: surface real error messages and fallback to previous provider state if unpushed --- .../src/app/store/providers/gitlab/gitlab.tsx | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/packages/tokens-studio-for-figma/src/app/store/providers/gitlab/gitlab.tsx b/packages/tokens-studio-for-figma/src/app/store/providers/gitlab/gitlab.tsx index c148e25ec..8c34a8357 100644 --- a/packages/tokens-studio-for-figma/src/app/store/providers/gitlab/gitlab.tsx +++ b/packages/tokens-studio-for-figma/src/app/store/providers/gitlab/gitlab.tsx @@ -30,11 +30,11 @@ type GitlabFormValues = Extract, { provider: Storag export const clientFactory = async (context: GitlabCredentials, isProUser: boolean) => { const { - secret, baseUrl, id: repoPathWithNamespace, filePath, branch, + secret, baseUrl, id: repoPathWithNamespace, filePath, branch, previousSourceBranch, } = context; const { repositoryId } = getRepositoryInformation(repoPathWithNamespace); - const storageClient = new GitlabTokenStorage(secret, repositoryId, repoPathWithNamespace, baseUrl ?? ''); + const storageClient = new GitlabTokenStorage(secret, repositoryId, repoPathWithNamespace, baseUrl ?? '', branch, previousSourceBranch); if (filePath) storageClient.changePath(filePath); if (branch) storageClient.selectBranch(branch); if (isProUser) storageClient.enableMultiFile(); @@ -44,7 +44,7 @@ export const clientFactory = async (context: GitlabCredentials, isProUser: boole export function useGitLab() { const tokens = useSelector(tokensSelector); const themes = useSelector(themesListSelector); - const localApiState = useSelector(localApiStateSelector); + const localApiState = useSelector(localApiStateSelector) as GitlabCredentials; const usedTokenSet = useSelector(usedTokenSetSelector); const activeTheme = useSelector(activeThemeSelector); const storeTokenIdInJsonEditor = useSelector(storeTokenIdInJsonEditorSelector); @@ -100,6 +100,8 @@ export function useGitLab() { themes, metadata, }); + const branches = await storage.fetchBranches(); + dispatch.branchState.setBranches(branches); const stringifiedRemoteTokens = JSON.stringify(compact([tokens, themes, TokenFormat.format]), null, 2); dispatch.tokenState.setLastSyncedState(stringifiedRemoteTokens); pushDialog({ state: 'success' }); @@ -112,16 +114,10 @@ export function useGitLab() { } catch (e: any) { closePushDialog(); console.log('Error pushing to GitLab', e); - if (e instanceof Error && e.message === ErrorMessages.GIT_MULTIFILE_PERMISSION_ERROR) { + if (e instanceof Error) { return { status: 'failure', - errorMessage: ErrorMessages.GIT_MULTIFILE_PERMISSION_ERROR, - }; - } - if (e instanceof Error && e.message === ErrorMessages.GITLAB_PUSH_TO_PROTECTED_BRANCH_ERROR) { - return { - status: 'failure', - errorMessage: ErrorMessages.GITLAB_PUSH_TO_PROTECTED_BRANCH_ERROR, + errorMessage: e.message, }; } return { @@ -132,8 +128,8 @@ export function useGitLab() { } return { status: 'success', - tokens, - themes, + tokens: {}, + themes: [], metadata: {}, }; }, [ @@ -273,7 +269,23 @@ export function useGitLab() { ]); const addNewGitLabCredentials = useCallback(async (context: GitlabFormValues): Promise => { + const previousBranch = localApiState.branch; + const previousFilePath = localApiState.filePath; + if (previousBranch !== context.branch) { + context = { ...context, previousSourceBranch: previousBranch }; + } const data = await syncTokensWithGitLab(context); + + // User cancelled pushing to the remote + if (data.status === 'success' && data.themes.length === 0) { + dispatch.uiState.setLocalApiState({ ...context, branch: previousBranch, filePath: previousFilePath }); + + return { + status: 'failure', + errorMessage: 'Push to remote cancelled!', + }; + } + if (data.status === 'success') { AsyncMessageChannel.ReactInstance.message({ type: AsyncMessageTypes.CREDENTIALS, From 7e8a791d93153e4aa1dd79f0d95e3ce3c0fc5bde Mon Sep 17 00:00:00 2001 From: Celia Usero Navarro Date: Tue, 20 Aug 2024 14:11:54 +0100 Subject: [PATCH 3/4] chore: update tests --- .../store/providers/__tests__/clientFactory.test.ts | 6 +++++- .../src/app/store/remoteTokens.test.ts | 10 +++++----- .../src/storage/__tests__/GitlabTokenStorage.test.ts | 5 +++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/tokens-studio-for-figma/src/app/store/providers/__tests__/clientFactory.test.ts b/packages/tokens-studio-for-figma/src/app/store/providers/__tests__/clientFactory.test.ts index 806465262..d1afcf345 100644 --- a/packages/tokens-studio-for-figma/src/app/store/providers/__tests__/clientFactory.test.ts +++ b/packages/tokens-studio-for-figma/src/app/store/providers/__tests__/clientFactory.test.ts @@ -21,15 +21,19 @@ describe('gitlab client factory', () => { const repositoryId = 'test-repo-id'; const secret = 'test-secret'; const baseUrl = 'test-url'; + const branch = 'develop'; + const previousSourceBranch = 'main'; const fullPath = `namespace/${repositoryId}`; const context = { id: fullPath, secret, baseUrl, + branch, + previousSourceBranch, } as unknown as GitlabCredentials; await clientFactory(context, false); - expect(GitlabTokenStorage).toHaveBeenCalledWith(secret, repositoryId, fullPath, baseUrl); + expect(GitlabTokenStorage).toHaveBeenCalledWith(secret, repositoryId, fullPath, baseUrl, branch, previousSourceBranch); }); it('should call change path if there is a filepath', async () => { diff --git a/packages/tokens-studio-for-figma/src/app/store/remoteTokens.test.ts b/packages/tokens-studio-for-figma/src/app/store/remoteTokens.test.ts index 5fa046747..f9a560fc0 100644 --- a/packages/tokens-studio-for-figma/src/app/store/remoteTokens.test.ts +++ b/packages/tokens-studio-for-figma/src/app/store/remoteTokens.test.ts @@ -703,7 +703,7 @@ describe('remoteTokens', () => { Object.values(contextMap).forEach((context) => { if (context === gitHubContext || context === gitLabContext || context === adoContext || context === bitbucketContext) { it(`Add newProviderItem to ${context.provider}, should push tokens and return status data if there is no content`, async () => { - mockFetchBranches.mockImplementationOnce(() => ( + mockFetchBranches.mockImplementation(() => ( Promise.resolve(['main']) )); mockRetrieve.mockImplementation(() => ( @@ -718,7 +718,7 @@ describe('remoteTokens', () => { await waitFor(() => { result.current.addNewProviderItem(context as StorageTypeCredentials); }); expect(mockPushDialog).toBeCalledTimes(2); expect(mockPushDialog.mock.calls[1][0].state).toBe('success'); - expect(await result.current.addNewProviderItem(context as StorageTypeCredentials)).toEqual(context === adoContext ? { errorMessage: 'Error syncing with ADO, check credentials', status: 'failure' } : { + expect(await result.current.addNewProviderItem(context as StorageTypeCredentials)).toEqual({ status: 'success', }); }); @@ -765,7 +765,7 @@ describe('remoteTokens', () => { expect(mockClosePushDialog).toBeCalledTimes(1); expect(await result.current.addNewProviderItem(context as StorageTypeCredentials)).toEqual({ status: 'failure', - errorMessage: context === adoContext ? ErrorMessages.GENERAL_CONNECTION_ERROR : errorMessageMap[contextName as keyof typeof errorMessageMap], + errorMessage: (context === adoContext || context === gitLabContext) ? ErrorMessages.GENERAL_CONNECTION_ERROR : errorMessageMap[contextName as keyof typeof errorMessageMap], }); }); } @@ -792,7 +792,7 @@ describe('remoteTokens', () => { Promise.resolve(true) )); await waitFor(() => { result.current.addNewProviderItem(context as StorageTypeCredentials); }); - if (context !== adoContext) { + if (context !== adoContext && context !== gitLabContext) { expect(notifyToUI).toBeCalledTimes(2); expect(notifyToUI).toBeCalledWith('No tokens stored on remote'); expect(await result.current.addNewProviderItem(context as StorageTypeCredentials)).toEqual({ @@ -800,7 +800,7 @@ describe('remoteTokens', () => { }); } else { expect(notifyToUI).toBeCalledTimes(1); - expect(notifyToUI).toBeCalledWith('Pulled tokens from ADO'); + expect(notifyToUI).toBeCalledWith(`Pulled tokens from ${contextName}`); expect(await result.current.addNewProviderItem(context as StorageTypeCredentials)).toEqual({ status: 'failure', errorMessage: 'Push to remote cancelled!', diff --git a/packages/tokens-studio-for-figma/src/storage/__tests__/GitlabTokenStorage.test.ts b/packages/tokens-studio-for-figma/src/storage/__tests__/GitlabTokenStorage.test.ts index d6f2dc27d..4f43fb3e4 100644 --- a/packages/tokens-studio-for-figma/src/storage/__tests__/GitlabTokenStorage.test.ts +++ b/packages/tokens-studio-for-figma/src/storage/__tests__/GitlabTokenStorage.test.ts @@ -15,6 +15,7 @@ const mockGetRepositoryFiles = jest.fn(); const mockCreateCommits = jest.fn(); const mockShowCommits = jest.fn(); const mockShowRepositoryFiles = jest.fn(); +const mockCreateRepositoryFiles = jest.fn(); jest.mock('@gitbeaker/rest', () => ({ Gitlab: jest.fn().mockImplementation(() => ({ @@ -41,6 +42,7 @@ jest.mock('@gitbeaker/rest', () => ({ RepositoryFiles: { showRaw: mockGetRepositoryFiles, show: mockShowRepositoryFiles, + create: mockCreateRepositoryFiles, }, Commits: { create: mockCreateCommits, @@ -455,7 +457,7 @@ describe('GitlabTokenStorage', () => { storeTokenIdInJsonEditor: true, }); - expect(mockCreateCommits).toBeCalledWith( + expect(mockCreateCommits).toHaveBeenCalledWith( 35102363, 'main', 'Initial commit', @@ -481,7 +483,6 @@ describe('GitlabTokenStorage', () => { filePath: 'data/tokens.json', }, ], - undefined, ); }); From 21d91656bcf75811df739921e433fdc380cde999 Mon Sep 17 00:00:00 2001 From: Celia Usero Navarro <114073780+cuserox@users.noreply.github.com> Date: Tue, 20 Aug 2024 17:33:33 +0100 Subject: [PATCH 4/4] Create beige-cameras-kneel.md --- .changeset/beige-cameras-kneel.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/beige-cameras-kneel.md diff --git a/.changeset/beige-cameras-kneel.md b/.changeset/beige-cameras-kneel.md new file mode 100644 index 000000000..d80a88dec --- /dev/null +++ b/.changeset/beige-cameras-kneel.md @@ -0,0 +1,5 @@ +--- +"@tokens-studio/figma-plugin": patch +--- + +Fixes issues when synchronizing data with GitLab, which prevented creating new branches on the fly and switching between single and multi-file setups.