Skip to content

Commit

Permalink
fix: allow GitLab edits to create & switch across new branches and so…
Browse files Browse the repository at this point in the history
…urce files (#3089)

* fix: create branch or source files in remote before pushing

* fix: surface real error messages and fallback to previous provider state if unpushed

* chore: update tests

* Create beige-cameras-kneel.md
  • Loading branch information
cuserox authored Aug 22, 2024
1 parent 8dfc3eb commit 1784f10
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 24 deletions.
5 changes: 5 additions & 0 deletions .changeset/beige-cameras-kneel.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ type GitlabFormValues = Extract<StorageTypeFormValues<false>, { 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();
Expand All @@ -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);
Expand Down Expand Up @@ -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' });
Expand All @@ -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 {
Expand All @@ -132,8 +128,8 @@ export function useGitLab() {
}
return {
status: 'success',
tokens,
themes,
tokens: {},
themes: [],
metadata: {},
};
}, [
Expand Down Expand Up @@ -273,7 +269,23 @@ export function useGitLab() {
]);

const addNewGitLabCredentials = useCallback(async (context: GitlabFormValues): Promise<RemoteResponseData> => {
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => (
Expand All @@ -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',
});
});
Expand Down Expand Up @@ -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],
});
});
}
Expand All @@ -792,15 +792,15 @@ 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({
status: 'success',
});
} 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!',
Expand Down
29 changes: 26 additions & 3 deletions packages/tokens-studio-for-figma/src/storage/GitlabTokenStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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() {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -235,9 +261,6 @@ export class GitlabTokenStorage extends GitTokenStorage {
branch,
message,
gitlabActions,
shouldCreateBranch ? {
startBranch: branches[0],
} : undefined,
);
return !!response;
} catch (e: any) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => ({
Expand All @@ -41,6 +42,7 @@ jest.mock('@gitbeaker/rest', () => ({
RepositoryFiles: {
showRaw: mockGetRepositoryFiles,
show: mockShowRepositoryFiles,
create: mockCreateRepositoryFiles,
},
Commits: {
create: mockCreateCommits,
Expand Down Expand Up @@ -455,7 +457,7 @@ describe('GitlabTokenStorage', () => {
storeTokenIdInJsonEditor: true,
});

expect(mockCreateCommits).toBeCalledWith(
expect(mockCreateCommits).toHaveBeenCalledWith(
35102363,
'main',
'Initial commit',
Expand All @@ -481,7 +483,6 @@ describe('GitlabTokenStorage', () => {
filePath: 'data/tokens.json',
},
],
undefined,
);
});

Expand Down
1 change: 1 addition & 0 deletions packages/tokens-studio-for-figma/src/types/StorageType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
>;

Expand Down

0 comments on commit 1784f10

Please sign in to comment.