Merge branch '233423-fix-ide-commit-to-new-branch-errors' into 'master'

Fix issues with IDE commit to new branch

See merge request gitlab-org/gitlab!51654

app/assets/javascripts/ide/components/commit_sidebar/radio_group.vue

@@ -74,6 +74,7 @@ export default {
       <input
         :placeholder="placeholderBranchName"
         :value="newBranchName"
+        data-testid="ide-new-branch-name"
         type="text"
         class="form-control monospace"
         @input="updateBranchName($event.target.value)"

app/assets/javascripts/ide/index.js

@@ -63,6 +63,10 @@ export function initIde(el, options = {}) {
         codesandboxBundlerUrl: el.dataset.codesandboxBundlerUrl,
       });
     },
+    beforeDestroy() {
+      // This helps tests do Singleton cleanups which we don't really have responsibility to know about here.
+      this.$emit('destroy');
+    },
     methods: {
       ...mapActions(['setEmptyStateSvgs', 'setLinks', 'setInitialData']),
     },

app/assets/javascripts/ide/stores/actions/project.js

@@ -44,8 +44,9 @@ export const refreshLastCommitData = ({ commit }, { projectId, branchId } = {})
         commit: data.commit,
       });
     })
-    .catch(() => {
+    .catch((e) => {
       flash(__('Error loading last commit.'), 'alert', document, null, false, true);
+      throw e;
     });
 
 export const createNewBranchFromDefault = ({ state, dispatch, getters }, branch) =>

app/assets/javascripts/ide/stores/modules/commit/actions.js

@@ -204,26 +204,25 @@ export const commitChanges = ({ commit, state, getters, dispatch, rootState, roo
           } else {
             dispatch('updateActivityBarView', leftSidebarViews.edit.name, { root: true });
             dispatch('updateViewer', 'editor', { root: true });
-
-            if (rootGetters.activeFile) {
-              dispatch(
-                'router/push',
-                `/project/${rootState.currentProjectId}/blob/${branchName}/-/${rootGetters.activeFile.path}`,
-                { root: true },
-              );
-            }
           }
         })
         .then(() => dispatch('updateCommitAction', consts.COMMIT_TO_CURRENT_BRANCH))
-        .then(() =>
-          dispatch(
+        .then(() => {
+          if (newBranch) {
+            const path = rootGetters.activeFile ? rootGetters.activeFile.path : '';
+
+            return dispatch(
+              'router/push',
+              `/project/${rootState.currentProjectId}/blob/${branchName}/-/${path}`,
+              { root: true },
+            );
+          }
+
+          return dispatch(
             'refreshLastCommitData',
-            {
-              projectId: rootState.currentProjectId,
-              branchId: rootState.currentBranchId,
-            },
+            { projectId: rootState.currentProjectId, branchId: branchName },
             { root: true },
-          ),
-        );
+          );
+        });
     });
 };

changelogs/unreleased/233423-fix-ide-commit-to-new-branch-errors.yml

+---
+title: Fix issues when Web IDE commits to new branch
+merge_request: 51654
+author:
+type: fixed

spec/frontend/ide/stores/modules/commit/actions_spec.js

@@ -19,6 +19,17 @@ jest.mock('~/lib/utils/url_utility', () => ({
 }));
 
 const TEST_COMMIT_SHA = '123456789';
+const COMMIT_RESPONSE = {
+  id: '123456',
+  short_id: '123',
+  message: 'test message',
+  committed_date: 'date',
+  parent_ids: [],
+  stats: {
+    additions: '1',
+    deletions: '2',
+  },
+};
 
 describe('IDE commit module actions', () => {
   let mock;
@@ -32,7 +43,9 @@ describe('IDE commit module actions', () => {
     mock = new MockAdapter(axios);
     jest.spyOn(router, 'push').mockImplementation();
 
-    mock.onGet('/api/v1/projects/abcproject/repository/branches/master').reply(200);
+    mock
+      .onGet('/api/v1/projects/abcproject/repository/branches/master')
+      .reply(200, { commit: COMMIT_RESPONSE });
   });
 
   afterEach(() => {
@@ -329,18 +342,6 @@ describe('IDE commit module actions', () => {
     });
 
     describe('success', () => {
-      const COMMIT_RESPONSE = {
-        id: '123456',
-        short_id: '123',
-        message: 'test message',
-        committed_date: 'date',
-        parent_ids: '321',
-        stats: {
-          additions: '1',
-          deletions: '2',
-        },
-      };
-
       beforeEach(() => {
         jest.spyOn(service, 'commit').mockResolvedValue({ data: COMMIT_RESPONSE });
       });
@@ -544,18 +545,6 @@ describe('IDE commit module actions', () => {
     });
 
     describe('first commit of a branch', () => {
-      const COMMIT_RESPONSE = {
-        id: '123456',
-        short_id: '123',
-        message: 'test message',
-        committed_date: 'date',
-        parent_ids: [],
-        stats: {
-          additions: '1',
-          deletions: '2',
-        },
-      };
-
       it('commits TOGGLE_EMPTY_STATE mutation on empty repo', (done) => {
         jest.spyOn(service, 'commit').mockResolvedValue({ data: COMMIT_RESPONSE });
         jest.spyOn(store, 'commit');

spec/frontend_integration/ide/helpers/ide_helper.js

@@ -138,6 +138,11 @@ export const createFile = async (path, content) => {
   await findAndSetEditorValue(content);
 };
 
+export const updateFile = async (path, content) => {
+  await openFile(path);
+  await findAndSetEditorValue(content);
+};
+
 export const getFilesList = () => {
   return screen.getAllByTestId('file-row-name-container').map((e) => e.textContent.trim());
 };
@@ -162,11 +167,33 @@ export const closeFile = async (path) => {
   button.click();
 };
 
-export const commit = async () => {
+/**
+ * Fill out and submit the commit form in the Web IDE
+ *
+ * @param {Object} options - Used to fill out the commit form in the IDE
+ * @param {Boolean} options.newBranch - Flag for the "Create a new branch" radio.
+ * @param {Boolean} options.newMR - Flag for the "Start a new merge request" checkbox.
+ * @param {String} options.newBranchName - Value to put in the new branch name input field. The Web IDE supports leaving this field blank.
+ */
+export const commit = async ({ newBranch = false, newMR = false, newBranchName = '' } = {}) => {
   switchLeftSidebarTab('Commit');
   screen.getByTestId('begin-commit-button').click();
 
-  await screen.findByLabelText(/Commit to .+ branch/).then((x) => x.click());
+  if (!newBranch) {
+    const option = await screen.findByLabelText(/Commit to .+ branch/);
+    option.click();
+  } else {
+    const option = await screen.findByLabelText('Create a new branch');
+    option.click();
+
+    const branchNameInput = await screen.findByTestId('ide-new-branch-name');
+    fireEvent.input(branchNameInput, { target: { value: newBranchName } });
+
+    const mrCheck = await screen.findByLabelText('Start a new merge request');
+    if (Boolean(mrCheck.checked) !== newMR) {
+      mrCheck.click();
+    }
+  }
 
   screen.getByText('Commit').click();
 };

spec/frontend_integration/ide/helpers/start.js

@@ -2,6 +2,7 @@ import { TEST_HOST } from 'helpers/test_constants';
 import extendStore from '~/ide/stores/extend';
 import { IDE_DATASET } from './mock_data';
 import { initIde } from '~/ide';
+import Editor from '~/ide/lib/editor';
 
 export default (container, { isRepoEmpty = false, path = '' } = {}) => {
   global.jsdom.reconfigure({
@@ -13,5 +14,16 @@ export default (container, { isRepoEmpty = false, path = '' } = {}) => {
   const el = document.createElement('div');
   Object.assign(el.dataset, IDE_DATASET);
   container.appendChild(el);
-  return initIde(el, { extendStore });
+  const vm = initIde(el, { extendStore });
+
+  // We need to dispose of editor Singleton things or tests will bump into eachother
+  vm.$on('destroy', () => {
+    if (Editor.editorInstance) {
+      Editor.editorInstance.modelManager.dispose();
+      Editor.editorInstance.dispose();
+      Editor.editorInstance = null;
+    }
+  });
+
+  return vm;
 };

spec/frontend_integration/ide/ide_integration_spec.js

@@ -55,6 +55,25 @@ describe('WebIDE', () => {
     });
   });
 
+  it('user commits changes to new branch', async () => {
+    vm = startWebIDE(container);
+
+    expect(window.location.pathname).toBe('/-/ide/project/gitlab-test/lorem-ipsum/tree/master/-/');
+
+    await ideHelper.updateFile('README.md', 'Lorem dolar si amit\n');
+    await ideHelper.commit({ newBranch: true, newMR: false, newBranchName: 'test-hello-world' });
+
+    await waitForText('All changes are committed');
+
+    // Wait for IDE to load new commit
+    await waitForText('10000000', document.querySelector('.ide-status-bar'));
+
+    // It's important that the new branch is now in the route
+    expect(window.location.pathname).toBe(
+      '/-/ide/project/gitlab-test/lorem-ipsum/blob/test-hello-world/-/README.md',
+    );
+  });
+
   it('user adds file that starts with +', async () => {
     vm = startWebIDE(container);
 

spec/frontend_integration/test_helpers/mock_server/routes/repository.js

@@ -37,13 +37,23 @@ export default (server) => {
     );
 
     const branch = schema.branches.findBy({ name: branchName });
+    const prevCommit = branch
+      ? branch.attrs.commit
+      : schema.branches.findBy({ name: 'master' }).attrs.commit;
 
     const commit = {
-      ...createNewCommit({ id: commitIdGenerator.next(), message }, branch.attrs.commit),
+      ...createNewCommit({ id: commitIdGenerator.next(), message }, prevCommit),
       __actions: actions,
     };
 
-    branch.update({ commit });
+    if (branch) {
+      branch.update({ commit });
+    } else {
+      schema.branches.create({
+        name: branchName,
+        commit,
+      });
+    }
 
     return commit;
   });