Merge branch 'himkp-33441' into 'master'

Stage all changes by default in Web IDE

See merge request gitlab-org/gitlab!21067

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

@@ -6,6 +6,7 @@ import CommitMessageField from './message_field.vue';
 import Actions from './actions.vue';
 import SuccessMessage from './success_message.vue';
 import { activityBarViews, MAX_WINDOW_HEIGHT_COMPACT } from '../../constants';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 
 export default {
   components: {
@@ -14,6 +15,7 @@ export default {
     CommitMessageField,
     SuccessMessage,
   },
+  mixins: [glFeatureFlagsMixin()],
   data() {
     return {
       isCompact: true,
@@ -27,9 +29,13 @@ export default {
     ...mapGetters('commit', ['discardDraftButtonDisabled', 'preBuiltCommitMessage']),
     overviewText() {
       return sprintf(
-        __(
-          '<strong>%{changedFilesLength} unstaged</strong> and <strong>%{stagedFilesLength} staged</strong> changes',
-        ),
+        this.glFeatures.stageAllByDefault
+          ? __(
+              '<strong>%{stagedFilesLength} staged</strong> and <strong>%{changedFilesLength} unstaged</strong> changes',
+            )
+          : __(
+              '<strong>%{changedFilesLength} unstaged</strong> and <strong>%{stagedFilesLength} staged</strong> changes',
+            ),
         {
           stagedFilesLength: this.stagedFiles.length,
           changedFilesLength: this.changedFiles.length,
@@ -39,6 +45,10 @@ export default {
     commitButtonText() {
       return this.stagedFiles.length ? __('Commit') : __('Stage & Commit');
     },
+
+    currentViewIsCommitView() {
+      return this.currentActivityView === activityBarViews.commit;
+    },
   },
   watch: {
     currentActivityView() {
@@ -46,27 +56,26 @@ export default {
         this.isCompact = false;
       } else {
         this.isCompact = !(
-          this.currentActivityView === activityBarViews.commit &&
-          window.innerHeight >= MAX_WINDOW_HEIGHT_COMPACT
+          this.currentViewIsCommitView && window.innerHeight >= MAX_WINDOW_HEIGHT_COMPACT
         );
       }
     },
-    lastCommitMsg() {
-      this.isCompact =
-        this.currentActivityView !== activityBarViews.commit && this.lastCommitMsg === '';
-    },
   },
   methods: {
     ...mapActions(['updateActivityBarView']),
     ...mapActions('commit', ['updateCommitMessage', 'discardDraft', 'commitChanges']),
-    toggleIsSmall() {
-      this.updateActivityBarView(activityBarViews.commit)
-        .then(() => {
-          this.isCompact = !this.isCompact;
-        })
-        .catch(e => {
-          throw e;
-        });
+    toggleIsCompact() {
+      if (this.currentViewIsCommitView) {
+        this.isCompact = !this.isCompact;
+      } else {
+        this.updateActivityBarView(activityBarViews.commit)
+          .then(() => {
+            this.isCompact = false;
+          })
+          .catch(e => {
+            throw e;
+          });
+      }
     },
     beforeEnterTransition() {
       const elHeight = this.isCompact
@@ -114,7 +123,7 @@ export default {
           :disabled="!hasChanges"
           type="button"
           class="btn btn-primary btn-sm btn-block qa-begin-commit-button"
-          @click="toggleIsSmall"
+          @click="toggleIsCompact"
         >
           {{ __('Commit…') }}
         </button>
@@ -148,7 +157,7 @@ export default {
             v-else
             type="button"
             class="btn btn-default btn-sm float-right"
-            @click="toggleIsSmall"
+            @click="toggleIsCompact"
           >
             {{ __('Collapse') }}
           </button>

app/assets/javascripts/ide/components/file_row_extra.vue

@@ -6,6 +6,7 @@ import Icon from '~/vue_shared/components/icon.vue';
 import ChangedFileIcon from '~/vue_shared/components/changed_file_icon.vue';
 import NewDropdown from './new_dropdown/index.vue';
 import MrFileIcon from './mr_file_icon.vue';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 
 export default {
   name: 'FileRowExtra',
@@ -18,6 +19,7 @@ export default {
     ChangedFileIcon,
     MrFileIcon,
   },
+  mixins: [glFeatureFlagsMixin()],
   props: {
     file: {
       type: Object,
@@ -55,10 +57,15 @@ export default {
         return n__('%d staged change', '%d staged changes', this.folderStagedCount);
       }
 
-      return sprintf(__('%{unstaged} unstaged and %{staged} staged changes'), {
-        unstaged: this.folderUnstagedCount,
-        staged: this.folderStagedCount,
-      });
+      return sprintf(
+        this.glFeatures.stageAllByDefault
+          ? __('%{staged} staged and %{unstaged} unstaged changes')
+          : __('%{unstaged} unstaged and %{staged} staged changes'),
+        {
+          unstaged: this.folderUnstagedCount,
+          staged: this.folderStagedCount,
+        },
+      );
     },
     showTreeChangesCount() {
       return this.isTree && this.changesCount > 0 && !this.file.opened;

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

@@ -51,7 +51,7 @@ export const setResizingStatus = ({ commit }, resizing) => {
 };
 
 export const createTempEntry = (
-  { state, commit, dispatch },
+  { state, commit, dispatch, getters },
   { name, type, content = '', base64 = false, binary = false, rawPath = '' },
 ) => {
   const fullName = name.slice(-1) !== '/' && type === 'tree' ? `${name}/` : name;
@@ -92,7 +92,11 @@ export const createTempEntry = (
 
   if (type === 'blob') {
     commit(types.TOGGLE_FILE_OPEN, file.path);
-    commit(types.ADD_FILE_TO_CHANGED, file.path);
+
+    if (gon.features?.stageAllByDefault)
+      commit(types.STAGE_CHANGE, { path: file.path, diffInfo: getters.getDiffInfo(file.path) });
+    else commit(types.ADD_FILE_TO_CHANGED, file.path);
+
     dispatch('setFileActive', file.path);
     dispatch('triggerFilesChange');
     dispatch('burstUnusedSeal');
@@ -238,7 +242,7 @@ export const deleteEntry = ({ commit, dispatch, state }, path) => {
 
 export const resetOpenFiles = ({ commit }) => commit(types.RESET_OPEN_FILES);
 
-export const renameEntry = ({ dispatch, commit, state }, { path, name, parentPath }) => {
+export const renameEntry = ({ dispatch, commit, state, getters }, { path, name, parentPath }) => {
   const entry = state.entries[path];
   const newPath = parentPath ? `${parentPath}/${name}` : name;
   const existingParent = parentPath && state.entries[parentPath];
@@ -268,7 +272,10 @@ export const renameEntry = ({ dispatch, commit, state }, { path, name, parentPat
     if (isReset) {
       commit(types.REMOVE_FILE_FROM_STAGED_AND_CHANGED, newEntry);
     } else if (!isInChanges) {
-      commit(types.ADD_FILE_TO_CHANGED, newPath);
+      if (gon.features?.stageAllByDefault)
+        commit(types.STAGE_CHANGE, { path: newPath, diffInfo: getters.getDiffInfo(newPath) });
+      else commit(types.ADD_FILE_TO_CHANGED, newPath);
+
       dispatch('burstUnusedSeal');
     }
 

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

@@ -147,7 +147,7 @@ export const getRawFileData = ({ state, commit, dispatch, getters }, { path }) =
   });
 };
 
-export const changeFileContent = ({ commit, dispatch, state }, { path, content }) => {
+export const changeFileContent = ({ commit, dispatch, state, getters }, { path, content }) => {
   const file = state.entries[path];
   commit(types.UPDATE_FILE_CONTENT, {
     path,
@@ -157,7 +157,9 @@ export const changeFileContent = ({ commit, dispatch, state }, { path, content }
   const indexOfChangedFile = state.changedFiles.findIndex(f => f.path === path);
 
   if (file.changed && indexOfChangedFile === -1) {
-    commit(types.ADD_FILE_TO_CHANGED, path);
+    if (gon.features?.stageAllByDefault)
+      commit(types.STAGE_CHANGE, { path, diffInfo: getters.getDiffInfo(path) });
+    else commit(types.ADD_FILE_TO_CHANGED, path);
   } else if (!file.changed && !file.tempFile && indexOfChangedFile !== -1) {
     commit(types.REMOVE_FILE_FROM_CHANGED, path);
   }

app/controllers/ide_controller.rb

@@ -3,6 +3,10 @@
 class IdeController < ApplicationController
   layout 'fullscreen'
 
+  before_action do
+    push_frontend_feature_flag(:stage_all_by_default, default_enabled: true)
+  end
+
   def index
     Gitlab::UsageDataCounters::WebIdeCounter.increment_views_count
   end

changelogs/unreleased/himkp-33441.yml

+---
+title: Stage all changes by default in Web IDE
+merge_request: 21067
+author:
+type: added

doc/user/project/web_ide/index.md

@@ -46,12 +46,16 @@ Single file editing is based on the [Ace Editor](https://ace.c9.io).
 ## Stage and commit changes
 
 After making your changes, click the **Commit** button in the bottom left to
-review the list of changed files. Click on each file to review the changes and
-click the tick icon to stage the file.
+review the list of changed files. If you're using GitLab 12.6 or older versions,
+click on each file to review the changes and tick the item to stage a file.
 
-![Review changes](img/review_changes_v12_3.png)
+From [GitLab 12.7 onwards](https://gitlab.com/gitlab-org/gitlab/issues/33441),
+all your files will be automatically staged. You still have the option to unstage
+changes in case you want to submit them in multiple smaller commits. To unstage
+a change, simply click the **Unstage** button when a staged file is open, or click
+the undo icon next to **Staged changes** to unstage all changes.
 
-Once you have staged some changes, you can add a commit message, commit the
+Once you have finalized your changes, you can add a commit message, commit the
 staged changes and directly create a merge request. In case you don't have write
 access to the selected branch, you will see a warning, but still be able to create
 a new branch and start a merge request.

locale/gitlab.pot

@@ -379,6 +379,9 @@ msgstr ""
 msgid "%{spanStart}in%{spanEnd} %{errorFn}"
 msgstr ""
 
+msgid "%{staged} staged and %{unstaged} unstaged changes"
+msgstr ""
+
 msgid "%{start} to %{end}"
 msgstr ""
 
@@ -700,6 +703,9 @@ msgstr ""
 msgid "<strong>%{pushes}</strong> pushes, more than <strong>%{commits}</strong> commits by <strong>%{people}</strong> contributors."
 msgstr ""
 
+msgid "<strong>%{stagedFilesLength} staged</strong> and <strong>%{changedFilesLength} unstaged</strong> changes"
+msgstr ""
+
 msgid "<strong>Deletes</strong> source branch"
 msgstr ""
 

qa/qa/page/project/web_ide/edit.rb

@@ -84,16 +84,13 @@ module QA
           end
 
           def commit_changes
-            # Clicking :begin_commit_button the first time switches from the
+            # Clicking :begin_commit_button switches from the
             # edit to the commit view
             click_element :begin_commit_button
             active_element? :commit_mode_tab
 
-            # We need to click :begin_commit_button again
-            click_element :begin_commit_button
-
-            # After clicking :begin_commit_button the 2nd time there is an
-            # animation that hides :begin_commit_button and shows :commit_button
+            # After clicking :begin_commit_button, there is an animation
+            # that hides :begin_commit_button and shows :commit_button
             #
             # Wait for the animation to complete before clicking :commit_button
             # otherwise the click will quietly do nothing.
@@ -102,9 +99,6 @@ module QA
                 has_element?(:commit_button)
             end
 
-            # At this point we're ready to commit and the button should be
-            # labelled "Stage & Commit"
-            #
             # Click :commit_button and keep retrying just in case part of the
             # animation is still in process even when the buttons have the
             # expected visibility.

spec/features/projects/tree/create_directory_spec.rb

@@ -46,8 +46,6 @@ describe 'Multi-file editor new directory', :js do
 
     find('.js-ide-commit-mode').click
 
-    click_button 'Stage'
-
     fill_in('commit-message', with: 'commit message ide')
 
     find(:css, ".js-ide-commit-new-mr input").set(false)

spec/features/projects/tree/create_file_spec.rb

@@ -36,8 +36,6 @@ describe 'Multi-file editor new file', :js do
 
     find('.js-ide-commit-mode').click
 
-    click_button 'Stage'
-
     fill_in('commit-message', with: 'commit message ide')
 
     find(:css, ".js-ide-commit-new-mr input").set(false)

spec/frontend/ide/stores/actions/file_spec.js

@@ -514,6 +514,8 @@ describe('IDE store file actions', () => {
 
   describe('changeFileContent', () => {
     let tmpFile;
+    const callAction = (content = 'content\n') =>
+      store.dispatch('changeFileContent', { path: tmpFile.path, content });
 
     beforeEach(() => {
       tmpFile = file('tmpFile');
@@ -523,11 +525,7 @@ describe('IDE store file actions', () => {
     });
 
     it('updates file content', done => {
-      store
-        .dispatch('changeFileContent', {
-          path: tmpFile.path,
-          content: 'content\n',
-        })
+      callAction()
         .then(() => {
           expect(tmpFile.content).toBe('content\n');
 
@@ -537,11 +535,7 @@ describe('IDE store file actions', () => {
     });
 
     it('adds a newline to the end of the file if it doesnt already exist', done => {
-      store
-        .dispatch('changeFileContent', {
-          path: tmpFile.path,
-          content: 'content',
-        })
+      callAction('content')
         .then(() => {
           expect(tmpFile.content).toBe('content\n');
 
@@ -551,11 +545,7 @@ describe('IDE store file actions', () => {
     });
 
     it('adds file into changedFiles array', done => {
-      store
-        .dispatch('changeFileContent', {
-          path: tmpFile.path,
-          content: 'content',
-        })
+      callAction()
         .then(() => {
           expect(store.state.changedFiles.length).toBe(1);
 
@@ -564,7 +554,7 @@ describe('IDE store file actions', () => {
         .catch(done.fail);
     });
 
-    it('adds file once into changedFiles array', done => {
+    it('adds file not more than once into changedFiles array', done => {
       store
         .dispatch('changeFileContent', {
           path: tmpFile.path,
@@ -604,6 +594,52 @@ describe('IDE store file actions', () => {
         .catch(done.fail);
     });
 
+    describe('when `gon.feature.stageAllByDefault` is true', () => {
+      const originalGonFeatures = Object.assign({}, gon.features);
+
+      beforeAll(() => {
+        gon.features = { stageAllByDefault: true };
+      });
+
+      afterAll(() => {
+        gon.features = originalGonFeatures;
+      });
+
+      it('adds file into stagedFiles array', done => {
+        store
+          .dispatch('changeFileContent', {
+            path: tmpFile.path,
+            content: 'content',
+          })
+          .then(() => {
+            expect(store.state.stagedFiles.length).toBe(1);
+
+            done();
+          })
+          .catch(done.fail);
+      });
+
+      it('adds file not more than once into stagedFiles array', done => {
+        store
+          .dispatch('changeFileContent', {
+            path: tmpFile.path,
+            content: 'content',
+          })
+          .then(() =>
+            store.dispatch('changeFileContent', {
+              path: tmpFile.path,
+              content: 'content 123',
+            }),
+          )
+          .then(() => {
+            expect(store.state.stagedFiles.length).toBe(1);
+
+            done();
+          })
+          .catch(done.fail);
+      });
+    });
+
     it('bursts unused seal', done => {
       store
         .dispatch('changeFileContent', {

spec/javascripts/ide/components/commit_sidebar/form_spec.js

@@ -33,6 +33,12 @@ describe('IDE commit form', () => {
   });
 
   describe('compact', () => {
+    beforeEach(done => {
+      vm.isCompact = true;
+
+      vm.$nextTick(done);
+    });
+
     it('renders commit button in compact mode', () => {
       expect(vm.$el.querySelector('.btn-primary')).not.toBeNull();
       expect(vm.$el.querySelector('.btn-primary').textContent).toContain('Commit');
@@ -61,7 +67,7 @@ describe('IDE commit form', () => {
       });
     });
 
-    it('toggles activity bar vie when clicking commit button', done => {
+    it('toggles activity bar view when clicking commit button', done => {
       vm.$el.querySelector('.btn-primary').click();
 
       vm.$nextTick(() => {
@@ -104,6 +110,17 @@ describe('IDE commit form', () => {
       });
     });
 
+    it('always opens itself in full view current activity view is not commit view when clicking commit button', done => {
+      vm.$el.querySelector('.btn-primary').click();
+
+      vm.$nextTick(() => {
+        expect(store.state.currentActivityView).toBe(activityBarViews.commit);
+        expect(vm.isCompact).toBe(false);
+
+        done();
+      });
+    });
+
     describe('discard draft button', () => {
       it('hidden when commitMessage is empty', () => {
         expect(vm.$el.querySelector('.btn-default').textContent).toContain('Collapse');

spec/javascripts/ide/stores/actions_spec.js

@@ -225,6 +225,35 @@ describe('Multi-file store actions', () => {
           .catch(done.fail);
       });
 
+      describe('when `gon.feature.stageAllByDefault` is true', () => {
+        const originalGonFeatures = Object.assign({}, gon.features);
+
+        beforeAll(() => {
+          gon.features = { stageAllByDefault: true };
+        });
+
+        afterAll(() => {
+          gon.features = originalGonFeatures;
+        });
+
+        it('adds tmp file to staged files', done => {
+          const name = 'test';
+
+          store
+            .dispatch('createTempEntry', {
+              name,
+              branchId: 'mybranch',
+              type: 'blob',
+            })
+            .then(() => {
+              expect(store.state.stagedFiles).toEqual([jasmine.objectContaining({ name })]);
+
+              done();
+            })
+            .catch(done.fail);
+        });
+      });
+
       it('adds tmp file to open files', done => {
         const name = 'test';
 
@@ -255,41 +284,25 @@ describe('Multi-file store actions', () => {
             type: 'blob',
           })
           .then(() => {
-            const f = store.state.entries[name];
-
-            expect(store.state.changedFiles.length).toBe(1);
-            expect(store.state.changedFiles[0].name).toBe(f.name);
+            expect(store.state.changedFiles).toEqual([
+              jasmine.objectContaining({ name, tempFile: true }),
+            ]);
 
             done();
           })
           .catch(done.fail);
       });
 
-      it('sets tmp file as active', done => {
-        testAction(
-          createTempEntry,
-          {
-            name: 'test',
-            branchId: 'mybranch',
-            type: 'blob',
-          },
-          store.state,
-          [
-            { type: types.CREATE_TMP_ENTRY, payload: jasmine.any(Object) },
-            { type: types.TOGGLE_FILE_OPEN, payload: 'test' },
-            { type: types.ADD_FILE_TO_CHANGED, payload: 'test' },
-          ],
-          jasmine.arrayContaining([
-            {
-              type: 'setFileActive',
-              payload: 'test',
-            },
-            {
-              type: 'triggerFilesChange',
-            },
-          ]),
-          done,
+      it('sets tmp file as active', () => {
+        const dispatch = jasmine.createSpy();
+        const commit = jasmine.createSpy();
+
+        createTempEntry(
+          { state: store.state, getters: store.getters, dispatch, commit },
+          { name: 'test', branchId: 'mybranch', type: 'blob' },
         );
+
+        expect(dispatch).toHaveBeenCalledWith('setFileActive', 'test');
       });
 
       it('creates flash message if file already exists', done => {
@@ -800,6 +813,33 @@ describe('Multi-file store actions', () => {
         });
       });
 
+      describe('when `gon.feature.stageAllByDefault` is true', () => {
+        const originalGonFeatures = Object.assign({}, gon.features);
+
+        beforeAll(() => {
+          gon.features = { stageAllByDefault: true };
+        });
+
+        afterAll(() => {
+          gon.features = originalGonFeatures;
+        });
+
+        it('by default renames an entry and stages it', () => {
+          const dispatch = jasmine.createSpy();
+          const commit = jasmine.createSpy();
+
+          renameEntry(
+            { dispatch, commit, state: store.state, getters: store.getters },
+            { path: 'orig', name: 'renamed' },
+          );
+
+          expect(commit.calls.allArgs()).toEqual([
+            [types.RENAME_ENTRY, { path: 'orig', name: 'renamed', parentPath: undefined }],
+            [types.STAGE_CHANGE, jasmine.objectContaining({ path: 'renamed' })],
+          ]);
+        });
+      });
+
       it('by default renames an entry and adds to changed', done => {
         testAction(
           renameEntry,
@@ -819,12 +859,12 @@ describe('Multi-file store actions', () => {
               payload: 'renamed',
             },
           ],
-          [{ type: 'burstUnusedSeal' }, { type: 'triggerFilesChange' }],
+          jasmine.any(Object),
           done,
         );
       });
 
-      it('if not changed, completely unstages entry if renamed to original', done => {
+      it('if not changed, completely unstages and discards entry if renamed to original', done => {
         testAction(
           renameEntry,
           { path: 'renamed', name: 'orig' },