Merge branch '196609-remove-staging' into 'master'

Remove staging from commit workflow in the Web IDE

See merge request gitlab-org/gitlab!26151

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

@@ -24,25 +24,19 @@ export default {
     discardModalTitle() {
       return sprintf(__('Discard changes to %{path}?'), { path: this.activeFile.path });
     },
-    actionButtonText() {
-      return this.activeFile.staged ? __('Unstage') : __('Stage');
-    },
     isStaged() {
       return !this.activeFile.changed && this.activeFile.staged;
     },
   },
   methods: {
     ...mapActions(['stageChange', 'unstageChange', 'discardFileChanges']),
-    actionButtonClicked() {
-      if (this.activeFile.staged) {
-        this.unstageChange(this.activeFile.path);
-      } else {
-        this.stageChange(this.activeFile.path);
-      }
-    },
     showDiscardModal() {
       this.$refs.discardModal.show();
     },
+    discardChanges(path) {
+      this.unstageChange(path);
+      this.discardFileChanges(path);
+    },
   },
 };
 </script>
@@ -65,19 +59,7 @@ export default {
         class="btn btn-remove btn-inverted append-right-8"
         @click="showDiscardModal"
       >
-        {{ __('Discard') }}
-      </button>
-      <button
-        ref="actionButton"
-        :class="{
-          'btn-success': !isStaged,
-          'btn-warning': isStaged,
-        }"
-        type="button"
-        class="btn btn-inverted"
-        @click="actionButtonClicked"
-      >
-        {{ actionButtonText }}
+        {{ __('Discard changes') }}
       </button>
     </div>
     <gl-modal
@@ -87,7 +69,7 @@ export default {
       :ok-title="__('Discard changes')"
       :modal-id="discardModalId"
       :title="discardModalTitle"
-      @ok="discardFileChanges(activeFile.path)"
+      @ok="discardChanges(activeFile.path)"
     >
       {{ __("You will lose all changes you've made to this file. This action cannot be undone.") }}
     </gl-modal>

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

 <script>
 import { mapState, mapActions, mapGetters } from 'vuex';
-import { sprintf, __ } from '~/locale';
+import { n__, __ } from '~/locale';
 import LoadingButton from '~/vue_shared/components/loading_button.vue';
 import CommitMessageField from './message_field.vue';
 import Actions from './actions.vue';
@@ -26,15 +26,7 @@ export default {
     ...mapGetters(['hasChanges']),
     ...mapGetters('commit', ['discardDraftButtonDisabled', 'preBuiltCommitMessage']),
     overviewText() {
-      return sprintf(
-        __(
-          '<strong>%{stagedFilesLength} staged</strong> and <strong>%{changedFilesLength} unstaged</strong> changes',
-        ),
-        {
-          stagedFilesLength: this.stagedFiles.length,
-          changedFilesLength: this.changedFiles.length,
-        },
-      );
+      return n__('%d changed file', '%d changed files', this.stagedFiles.length);
     },
     commitButtonText() {
       return this.stagedFiles.length ? __('Commit') : __('Stage & Commit');
@@ -125,7 +117,7 @@ export default {
         >
           {{ __('Commit…') }}
         </button>
-        <p class="text-center" v-html="overviewText"></p>
+        <p class="text-center bold">{{ overviewText }}</p>
       </div>
       <form v-if="!isCompact" ref="formEl" @submit.prevent.stop="commitChanges">
         <transition name="fade"> <success-message v-show="lastCommitMsg" /> </transition>

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

@@ -17,10 +17,6 @@ export default {
     tooltip,
   },
   props: {
-    title: {
-      type: String,
-      required: true,
-    },
     fileList: {
       type: Array,
       required: true,
@@ -29,18 +25,6 @@ export default {
       type: String,
       required: true,
     },
-    action: {
-      type: String,
-      required: true,
-    },
-    actionBtnText: {
-      type: String,
-      required: true,
-    },
-    actionBtnIcon: {
-      type: String,
-      required: true,
-    },
     stagedList: {
       type: Boolean,
       required: false,
@@ -63,9 +47,9 @@ export default {
   },
   computed: {
     titleText() {
-      return sprintf(__('%{title} changes'), {
-        title: this.title,
-      });
+      if (!this.title) return __('Changes');
+
+      return sprintf(__('%{title} changes'), { title: this.title });
     },
     filesLength() {
       return this.fileList.length;
@@ -73,17 +57,16 @@ export default {
   },
   methods: {
     ...mapActions(['stageAllChanges', 'unstageAllChanges', 'discardAllChanges']),
-    actionBtnClicked() {
-      this[this.action]();
-
-      $(this.$refs.actionBtn).tooltip('hide');
-    },
     openDiscardModal() {
       $('#discard-all-changes').modal('show');
     },
+    unstageAndDiscardAllChanges() {
+      this.unstageAllChanges();
+      this.discardAllChanges();
+    },
   },
   discardModalText: __(
-    "You will lose all the unstaged changes you've made in this project. This action cannot be undone.",
+    "You will lose all uncommitted changes you've made in this project. This action cannot be undone.",
   ),
 };
 </script>
@@ -95,24 +78,6 @@ export default {
         <icon v-once :name="iconName" :size="18" class="append-right-8" />
         <strong> {{ titleText }} </strong>
         <div class="d-flex ml-auto">
-          <button
-            ref="actionBtn"
-            v-tooltip
-            :title="actionBtnText"
-            :aria-label="actionBtnText"
-            :disabled="!filesLength"
-            :class="{
-              'disabled-content': !filesLength,
-            }"
-            type="button"
-            class="d-flex ide-staged-action-btn p-0 border-0 align-items-center"
-            data-placement="bottom"
-            data-container="body"
-            data-boundary="viewport"
-            @click="actionBtnClicked"
-          >
-            <icon :name="actionBtnIcon" :size="16" class="ml-auto mr-auto" />
-          </button>
           <button
             v-if="!stagedList"
             v-tooltip
@@ -151,9 +116,9 @@ export default {
       v-if="!stagedList"
       id="discard-all-changes"
       :footer-primary-button-text="__('Discard all changes')"
-      :header-title-text="__('Discard all unstaged changes?')"
+      :header-title-text="__('Discard all changes?')"
       footer-primary-button-variant="danger"
-      @submit="discardAllChanges"
+      @submit="unstageAndDiscardAllChanges"
     >
       {{ $options.discardModalText }}
     </gl-modal>

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

@@ -57,13 +57,7 @@ export default {
     },
   },
   methods: {
-    ...mapActions([
-      'discardFileChanges',
-      'updateViewer',
-      'openPendingTab',
-      'unstageChange',
-      'stageChange',
-    ]),
+    ...mapActions(['discardFileChanges', 'updateViewer', 'openPendingTab']),
     openFileInEditor() {
       if (this.file.type === 'tree') return null;
 
@@ -76,13 +70,6 @@ export default {
         }
       });
     },
-    fileAction() {
-      if (this.file.staged) {
-        this.unstageChange(this.file.path);
-      } else {
-        this.stageChange(this.file.path);
-      }
-    },
   },
 };
 </script>
@@ -97,7 +84,6 @@ export default {
       }"
       class="multi-file-commit-list-path w-100 border-0 ml-0 mr-0"
       role="button"
-      @dblclick="fileAction"
       @click="openFileInEditor"
     >
       <span class="multi-file-commit-list-file-path d-flex align-items-center">

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

 <script>
 import { mapGetters } from 'vuex';
-import { n__, __, sprintf } from '~/locale';
+import { n__ } from '~/locale';
 import tooltip from '~/vue_shared/directives/tooltip';
 import Icon from '~/vue_shared/components/icon.vue';
 import ChangedFileIcon from '~/vue_shared/components/changed_file_icon.vue';
@@ -49,16 +49,7 @@ export default {
     folderChangesTooltip() {
       if (this.changesCount === 0) return undefined;
 
-      if (this.folderUnstagedCount > 0 && this.folderStagedCount === 0) {
-        return n__('%d unstaged change', '%d unstaged changes', this.folderUnstagedCount);
-      } else if (this.folderUnstagedCount === 0 && this.folderStagedCount > 0) {
-        return n__('%d staged change', '%d staged changes', this.folderStagedCount);
-      }
-
-      return sprintf(__('%{staged} staged and %{unstaged} unstaged changes'), {
-        unstaged: this.folderUnstagedCount,
-        staged: this.folderStagedCount,
-      });
+      return n__('%d changed file', '%d changed files', this.changesCount);
     },
     showTreeChangesCount() {
       return this.isTree && this.changesCount > 0 && !this.file.opened;

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

@@ -86,28 +86,12 @@ export default {
     </deprecated-modal>
     <template v-if="showStageUnstageArea">
       <commit-files-list
-        :title="__('Unstaged')"
-        :key-prefix="$options.stageKeys.unstaged"
-        :file-list="changedFiles"
-        :action-btn-text="__('Stage all changes')"
-        :active-file-key="activeFileKey"
-        :empty-state-text="__('There are no unstaged changes')"
-        action="stageAllChanges"
-        action-btn-icon="stage-all"
-        class="is-first"
-        icon-name="unstaged"
-      />
-      <commit-files-list
-        :title="__('Staged')"
         :key-prefix="$options.stageKeys.staged"
         :file-list="stagedFiles"
-        :action-btn-text="__('Unstage all changes')"
-        :staged-list="true"
         :active-file-key="activeFileKey"
-        :empty-state-text="__('There are no staged changes')"
-        action="unstageAllChanges"
-        action-btn-icon="unstage-all"
-        icon-name="staged"
+        :empty-state-text="__('There are no changes')"
+        class="is-first"
+        icon-name="unstaged"
       />
     </template>
     <empty-state v-if="unusedSeal" />

app/assets/javascripts/vue_shared/components/changed_file_icon.vue

 <script>
 import { GlTooltipDirective } from '@gitlab/ui';
 import Icon from '~/vue_shared/components/icon.vue';
-import { __, sprintf } from '~/locale';
+import { __ } from '~/locale';
 import { getCommitIconMap } from '~/ide/utils';
 
 export default {
@@ -51,17 +51,7 @@ export default {
     tooltipTitle() {
       if (!this.showTooltip || !this.file.changed) return undefined;
 
-      const type = this.file.tempFile ? 'addition' : 'modification';
-
-      if (this.file.staged) {
-        return sprintf(__('Staged %{type}'), {
-          type,
-        });
-      }
-
-      return sprintf(__('Unstaged %{type}'), {
-        type,
-      });
+      return this.file.tempFile ? __('Added') : __('Modified');
     },
     showIcon() {
       return (

changelogs/unreleased/196609-remove-staging.yml

+---
+title: Remove staging from commit workflow in the Web IDE
+merge_request: 26151
+author:
+type: removed

doc/user/project/web_ide/index.md

@@ -43,30 +43,33 @@ you can find a more complete list of supported languages in the
 NOTE: **Note:**
 Single file editing is based on the [Ace Editor](https://ace.c9.io).
 
-## Stage and commit changes
+## Commit changes
 
-After making your changes, click the **Commit** button in the bottom left to
-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.
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/4539) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 10.4 and [brought to GitLab Core](https://gitlab.com/gitlab-org/gitlab-foss/issues/44157) in 10.7.
+> - From [GitLab 12.7 onwards](https://gitlab.com/gitlab-org/gitlab/issues/33441),
+files were automatically staged.
+> - From [GitLab 12.9 onwards](https://gitlab.com/gitlab-org/gitlab/-/issues/196609), support for staging files was removed
+to prevent loss of unstaged data. All your current changes necessarily have to be
+committed or discarded.
 
-From [GitLab 12.7 onward](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.
+After making your changes, click the **Commit** button on the bottom-left to
+review the list of changed files.
 
 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
+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.
 
-![Commit changes](img/commit_changes_v12_3.png)
+To discard a change in a particular file, click the **Discard changes** button on that
+file in the changes tab. To discard all the changes, click the trash icon on the
+top-right corner of the changes sidebar.
+
+![Commit changes](img/commit_changes_v12_9.png)
 
 ## Reviewing changes
 
 Before you commit your changes, you can compare them with the previous commit
-by switching to the review mode or selecting the file from the staged files
-list.
+by switching to the review mode or selecting the file from the list of changes.
 
 An additional review mode is available when you open a merge request, which
 shows you a preview of the merge request diff if you commit your changes.

locale/gitlab.pot

@@ -68,6 +68,11 @@ msgstr ""
 msgid "\"%{path}\" did not exist on \"%{ref}\""
 msgstr ""
 
+msgid "%d changed file"
+msgid_plural "%d changed files"
+msgstr[0] ""
+msgstr[1] ""
+
 msgid "%d code quality issue"
 msgid_plural "%d code quality issues"
 msgstr[0] ""
@@ -186,21 +191,11 @@ msgid_plural "%d shards selected"
 msgstr[0] ""
 msgstr[1] ""
 
-msgid "%d staged change"
-msgid_plural "%d staged changes"
-msgstr[0] ""
-msgstr[1] ""
-
 msgid "%d tag"
 msgid_plural "%d tags"
 msgstr[0] ""
 msgstr[1] ""
 
-msgid "%d unstaged change"
-msgid_plural "%d unstaged changes"
-msgstr[0] ""
-msgstr[1] ""
-
 msgid "%d vulnerability dismissed"
 msgid_plural "%d vulnerabilities dismissed"
 msgstr[0] ""
@@ -431,9 +426,6 @@ msgstr ""
 msgid "%{spanStart}in%{spanEnd} %{errorFn}"
 msgstr ""
 
-msgid "%{staged} staged and %{unstaged} unstaged changes"
-msgstr ""
-
 msgid "%{start} to %{end}"
 msgstr ""
 
@@ -777,9 +769,6 @@ msgstr ""
 msgid "<strong>%{group_name}</strong> group members"
 msgstr ""
 
-msgid "<strong>%{stagedFilesLength} staged</strong> and <strong>%{changedFilesLength} unstaged</strong> changes"
-msgstr ""
-
 msgid "<strong>Deletes</strong> source branch"
 msgstr ""
 
@@ -6881,13 +6870,10 @@ msgstr ""
 msgid "Disabled mirrors can only be enabled by instance owners. It is recommended that you delete them."
 msgstr ""
 
-msgid "Discard"
-msgstr ""
-
 msgid "Discard all changes"
 msgstr ""
 
-msgid "Discard all unstaged changes?"
+msgid "Discard all changes?"
 msgstr ""
 
 msgid "Discard changes"
@@ -12765,6 +12751,9 @@ msgstr ""
 msgid "Modal|Close"
 msgstr ""
 
+msgid "Modified"
+msgstr ""
+
 msgid "Modified in this version"
 msgstr ""
 
@@ -18698,21 +18687,12 @@ msgstr ""
 msgid "Stage & Commit"
 msgstr ""
 
-msgid "Stage all changes"
-msgstr ""
-
 msgid "Stage data updated"
 msgstr ""
 
 msgid "Stage removed"
 msgstr ""
 
-msgid "Staged"
-msgstr ""
-
-msgid "Staged %{type}"
-msgstr ""
-
 msgid "Star a label to make it a priority label. Order the prioritized labels to change their relative priority, by dragging."
 msgstr ""
 
@@ -19869,6 +19849,9 @@ msgstr ""
 msgid "There are no archived projects yet"
 msgstr ""
 
+msgid "There are no changes"
+msgstr ""
+
 msgid "There are no charts configured for this page"
 msgstr ""
 
@@ -19905,12 +19888,6 @@ msgstr ""
 msgid "There are no projects shared with this group yet"
 msgstr ""
 
-msgid "There are no staged changes"
-msgstr ""
-
-msgid "There are no unstaged changes"
-msgstr ""
-
 msgid "There is a limit of %{ci_project_subscriptions_limit} subscriptions from or to a project."
 msgstr ""
 
@@ -21322,18 +21299,6 @@ msgstr ""
 msgid "Unschedule job"
 msgstr ""
 
-msgid "Unstage"
-msgstr ""
-
-msgid "Unstage all changes"
-msgstr ""
-
-msgid "Unstaged"
-msgstr ""
-
-msgid "Unstaged %{type}"
-msgstr ""
-
 msgid "Unstar"
 msgstr ""
 
@@ -23006,7 +22971,7 @@ msgstr ""
 msgid "You will lose all changes you've made to this file. This action cannot be undone."
 msgstr ""
 
-msgid "You will lose all the unstaged changes you've made in this project. This action cannot be undone."
+msgid "You will lose all uncommitted changes you've made in this project. This action cannot be undone."
 msgstr ""
 
 msgid "You will need to update your local repositories to point to the new location."

spec/frontend/ide/components/commit_sidebar/editor_header_spec.js

@@ -14,7 +14,6 @@ describe('IDE commit editor header', () => {
 
   const findDiscardModal = () => wrapper.find({ ref: 'discardModal' });
   const findDiscardButton = () => wrapper.find({ ref: 'discardButton' });
-  const findActionButton = () => wrapper.find({ ref: 'actionButton' });
 
   beforeEach(() => {
     f = file('file');
@@ -28,9 +27,7 @@ describe('IDE commit editor header', () => {
       },
     });
 
-    jest.spyOn(wrapper.vm, 'stageChange').mockImplementation();
-    jest.spyOn(wrapper.vm, 'unstageChange').mockImplementation();
-    jest.spyOn(wrapper.vm, 'discardFileChanges').mockImplementation();
+    jest.spyOn(wrapper.vm, 'discardChanges').mockImplementation();
   });
 
   afterEach(() => {
@@ -38,8 +35,8 @@ describe('IDE commit editor header', () => {
     wrapper = null;
   });
 
-  it('renders button to discard & stage', () => {
-    expect(wrapper.vm.$el.querySelectorAll('.btn').length).toBe(2);
+  it('renders button to discard', () => {
+    expect(wrapper.vm.$el.querySelectorAll('.btn')).toHaveLength(1);
   });
 
   describe('discard button', () => {
@@ -60,23 +57,7 @@ describe('IDE commit editor header', () => {
     it('calls discardFileChanges if dialog result is confirmed', () => {
       modal.vm.$emit('ok');
 
-      expect(wrapper.vm.discardFileChanges).toHaveBeenCalledWith(f.path);
-    });
-  });
-
-  describe('stage/unstage button', () => {
-    it('unstages the file if it was already staged', () => {
-      f.staged = true;
-
-      findActionButton().trigger('click');
-
-      expect(wrapper.vm.unstageChange).toHaveBeenCalledWith(f.path);
-    });
-
-    it('stages the file if it was not staged', () => {
-      findActionButton().trigger('click');
-
-      expect(wrapper.vm.stageChange).toHaveBeenCalledWith(f.path);
+      expect(wrapper.vm.discardChanges).toHaveBeenCalledWith(f.path);
     });
   });
 });

spec/frontend/vue_shared/components/changed_file_icon_spec.js

@@ -55,9 +55,9 @@ describe('Changed file icon', () => {
 
   describe.each`
     file             | iconName                 | tooltipText   | desc
-    ${changedFile()} | ${'file-modified'}       | ${'Unstaged modification'} | ${'with file changed'}
-    ${stagedFile()}  | ${'file-modified-solid'} | ${'Staged modification'}   | ${'with file staged'}
-    ${newFile()}     | ${'file-addition'}       | ${'Unstaged addition'}     | ${'with file new'}
+    ${changedFile()} | ${'file-modified'}       | ${'Modified'} | ${'with file changed'}
+    ${stagedFile()}  | ${'file-modified-solid'} | ${'Modified'} | ${'with file staged'}
+    ${newFile()}     | ${'file-addition'}       | ${'Added'}    | ${'with file new'}
   `('$desc', ({ file, iconName, tooltipText }) => {
     beforeEach(() => {
       factory({ file });

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

@@ -52,7 +52,7 @@ describe('IDE commit form', () => {
       vm.$store.state.stagedFiles.push('test');
 
       vm.$nextTick(() => {
-        expect(vm.$el.querySelector('p').textContent).toContain('1 staged and 1 unstaged changes');
+        expect(vm.$el.querySelector('p').textContent).toContain('1 changed file');
         done();
       });
     });

spec/javascripts/ide/components/file_row_extra_spec.js

@@ -41,30 +41,20 @@ describe('IDE extra file row component', () => {
 
   describe('folderChangesTooltip', () => {
     it('returns undefined when changes count is 0', () => {
-      expect(vm.folderChangesTooltip).toBe(undefined);
-    });
-
-    it('returns unstaged changes text', () => {
-      changesCount = 1;
-      unstagedFilesCount = 1;
-
-      expect(vm.folderChangesTooltip).toBe('1 unstaged change');
-    });
-
-    it('returns staged changes text', () => {
-      changesCount = 1;
-      stagedFilesCount = 1;
+      changesCount = 0;
 
-      expect(vm.folderChangesTooltip).toBe('1 staged change');
+      expect(vm.folderChangesTooltip).toBe(undefined);
     });
 
-    it('returns staged and unstaged changes text', () => {
-      changesCount = 1;
-      stagedFilesCount = 1;
-      unstagedFilesCount = 1;
+    [{ input: 1, output: '1 changed file' }, { input: 2, output: '2 changed files' }].forEach(
+      ({ input, output }) => {
+        it('returns changed files count if changes count is not 0', () => {
+          changesCount = input;
 
-      expect(vm.folderChangesTooltip).toBe('1 staged and 1 unstaged changes');
+          expect(vm.folderChangesTooltip).toBe(output);
         });
+      },
+    );
   });
 
   describe('show tree changes count', () => {

spec/javascripts/ide/components/repo_commit_section_spec.js

@@ -30,19 +30,13 @@ describe('RepoCommitSection', () => {
     const files = [file('file1'), file('file2')].map(f =>
       Object.assign(f, {
         type: 'blob',
+        content: 'orginal content',
       }),
     );
 
     vm.$store.state.rightPanelCollapsed = false;
     vm.$store.state.currentBranch = 'master';
-    vm.$store.state.changedFiles = [...files];
-    vm.$store.state.changedFiles.forEach(f =>
-      Object.assign(f, {
-        changed: true,
-        content: 'changedFile testing',
-      }),
-    );
-
+    vm.$store.state.changedFiles = [];
     vm.$store.state.stagedFiles = [{ ...files[0] }, { ...files[1] }];
     vm.$store.state.stagedFiles.forEach(f =>
       Object.assign(f, {
@@ -51,7 +45,7 @@ describe('RepoCommitSection', () => {
       }),
     );
 
-    vm.$store.state.changedFiles.forEach(f => {
+    files.forEach(f => {
       vm.$store.state.entries[f.path] = f;
     });
 
@@ -96,7 +90,7 @@ describe('RepoCommitSection', () => {
     const changedFileElements = [...vm.$el.querySelectorAll('.multi-file-commit-list > li')];
     const allFiles = vm.$store.state.changedFiles.concat(vm.$store.state.stagedFiles);
 
-    expect(changedFileElements.length).toEqual(4);
+    expect(changedFileElements).toHaveLength(2);
 
     changedFileElements.forEach((changedFile, i) => {
       expect(changedFile.textContent.trim()).toContain(allFiles[i].path);