diff options
67 files changed, 1941 insertions, 529 deletions
@@ -140,7 +140,7 @@ gem 'creole', '~> 0.5.0' gem 'wikicloth', '0.8.1' gem 'asciidoctor', '~> 1.5.6' gem 'asciidoctor-plantuml', '0.0.8' -gem 'rouge', '~> 2.0' +gem 'rouge', '~> 3.1' gem 'truncato', '~> 0.7.9' gem 'bootstrap_form', '~> 2.7.0' gem 'nokogiri', '~> 1.8.2' diff --git a/Gemfile.lock b/Gemfile.lock index d6ea73da805..745732f3537 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -303,12 +303,12 @@ GEM flowdock (~> 0.7) gitlab-grit (>= 2.4.1) multi_json - gitlab-gollum-lib (4.2.7.1) + gitlab-gollum-lib (4.2.7.2) gemojione (~> 3.2) github-markup (~> 1.6) gollum-grit_adapter (~> 1.0) nokogiri (>= 1.6.1, < 2.0) - rouge (~> 2.1) + rouge (~> 3.1) sanitize (~> 2.1) stringex (~> 2.6) gitlab-gollum-rugged_adapter (0.4.4) @@ -747,7 +747,7 @@ GEM retriable (3.1.1) rinku (2.0.0) rotp (2.1.2) - rouge (2.2.1) + rouge (3.1.1) rqrcode (0.7.0) chunky_png rqrcode-rails3 (0.1.7) @@ -1160,7 +1160,7 @@ DEPENDENCIES redis-rails (~> 5.0.2) request_store (~> 1.3) responders (~> 2.0) - rouge (~> 2.0) + rouge (~> 3.1) rqrcode-rails3 (~> 0.1.7) rspec-parameterized rspec-rails (~> 3.6.0) diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index c953b9708a0..a0330cbdd02 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -69,7 +69,7 @@ GEM unf ast (2.4.0) atomic (1.1.100) - attr_encrypted (3.0.3) + attr_encrypted (3.1.0) encryptor (~> 3.0.0) attr_required (1.0.1) autoprefixer-rails (8.1.0.1) @@ -291,9 +291,9 @@ GEM po_to_json (>= 1.0.0) rails (>= 3.2.0) gherkin-ruby (0.3.2) - gitaly-proto (0.94.0) + gitaly-proto (0.97.0) google-protobuf (~> 3.1) - grpc (~> 1.0) + grpc (~> 1.10) github-linguist (5.3.3) charlock_holmes (~> 0.7.5) escape_utils (~> 1.1.0) @@ -304,6 +304,17 @@ GEM flowdock (~> 0.7) gitlab-grit (>= 2.4.1) multi_json + gitlab-gollum-lib (4.2.7.1) + gemojione (~> 3.2) + github-markup (~> 1.6) + gollum-grit_adapter (~> 1.0) + nokogiri (>= 1.6.1, < 2.0) + rouge (~> 2.1) + sanitize (~> 2.1) + stringex (~> 2.6) + gitlab-gollum-rugged_adapter (0.4.4) + mime-types (>= 1.15) + rugged (~> 0.25) gitlab-grit (2.8.2) charlock_holmes (~> 0.6) diff-lcs (~> 1.1) @@ -321,22 +332,8 @@ GEM rubyntlm (~> 0.5) globalid (0.4.1) activesupport (>= 4.2.0) - goldiloader (2.0.1) - activerecord (>= 4.2, < 5.2) - activesupport (>= 4.2, < 5.2) gollum-grit_adapter (1.0.1) gitlab-grit (~> 2.7, >= 2.7.1) - gollum-lib (4.2.7) - gemojione (~> 3.2) - github-markup (~> 1.6) - gollum-grit_adapter (~> 1.0) - nokogiri (>= 1.6.1, < 2.0) - rouge (~> 2.1) - sanitize (~> 2.1) - stringex (~> 2.6) - gollum-rugged_adapter (0.4.4) - mime-types (>= 1.15) - rugged (~> 0.25) gon (6.1.0) actionpack (>= 3.0) json @@ -1009,7 +1006,7 @@ DEPENDENCIES asciidoctor (~> 1.5.6) asciidoctor-plantuml (= 0.0.8) asset_sync (~> 2.2.0) - attr_encrypted (~> 3.0.0) + attr_encrypted (~> 3.1.0) awesome_print (~> 1.2.0) babosa (~> 1.0.2) base32 (~> 0.3.0) @@ -1069,15 +1066,14 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.94.0) + gitaly-proto (~> 0.97.0) github-linguist (~> 5.3.3) gitlab-flowdock-git-hook (~> 1.0.1) + gitlab-gollum-lib (~> 4.2) + gitlab-gollum-rugged_adapter (~> 0.4.4) gitlab-markup (~> 1.6.2) gitlab-styles (~> 2.3) gitlab_omniauth-ldap (~> 2.0.4) - goldiloader (~> 2.0) - gollum-lib (~> 4.2) - gollum-rugged_adapter (~> 0.4.4) gon (~> 6.1.0) google-api-client (~> 0.19.8) google-protobuf (= 3.5.1) diff --git a/app/assets/javascripts/ide/components/changed_file_icon.vue b/app/assets/javascripts/ide/components/changed_file_icon.vue index 037e3efb4ce..1fc11c84639 100644 --- a/app/assets/javascripts/ide/components/changed_file_icon.vue +++ b/app/assets/javascripts/ide/components/changed_file_icon.vue @@ -1,31 +1,87 @@ <script> -import icon from '~/vue_shared/components/icon.vue'; +import tooltip from '~/vue_shared/directives/tooltip'; +import Icon from '~/vue_shared/components/icon.vue'; +import { pluralize } from '~/lib/utils/text_utility'; +import { __, sprintf } from '~/locale'; export default { components: { - icon, + Icon, + }, + directives: { + tooltip, }, props: { file: { type: Object, required: true, }, + showTooltip: { + type: Boolean, + required: false, + default: false, + }, + showStagedIcon: { + type: Boolean, + required: false, + default: false, + }, }, computed: { changedIcon() { - return this.file.tempFile ? 'file-addition' : 'file-modified'; + const suffix = this.file.staged && !this.showStagedIcon ? '-solid' : ''; + return this.file.tempFile ? `file-addition${suffix}` : `file-modified${suffix}`; + }, + stagedIcon() { + return `${this.changedIcon}-solid`; }, changedIconClass() { - return `multi-${this.changedIcon}`; + return `multi-${this.changedIcon} prepend-left-5 pull-left`; + }, + tooltipTitle() { + if (!this.showTooltip) return undefined; + + const type = this.file.tempFile ? 'addition' : 'modification'; + + if (this.file.changed && !this.file.staged) { + return sprintf(__('Unstaged %{type}'), { + type, + }); + } else if (!this.file.changed && this.file.staged) { + return sprintf(__('Staged %{type}'), { + type, + }); + } else if (this.file.changed && this.file.staged) { + return sprintf(__('Unstaged and staged %{type}'), { + type: pluralize(type), + }); + } + + return undefined; }, }, }; </script> <template> - <icon - :name="changedIcon" - :size="12" - :css-classes="`ide-file-changed-icon ${changedIconClass}`" - /> + <span + v-tooltip + :title="tooltipTitle" + data-container="body" + data-placement="right" + class="ide-file-changed-icon" + > + <icon + v-if="file.staged && showStagedIcon" + :name="stagedIcon" + :size="12" + :css-classes="changedIconClass" + /> + <icon + v-if="file.changed || file.tempFile || (file.staged && !showStagedIcon)" + :name="changedIcon" + :size="12" + :css-classes="changedIconClass" + /> + </span> </template> diff --git a/app/assets/javascripts/ide/components/commit_sidebar/empty_state.vue b/app/assets/javascripts/ide/components/commit_sidebar/empty_state.vue new file mode 100644 index 00000000000..6424b93ce54 --- /dev/null +++ b/app/assets/javascripts/ide/components/commit_sidebar/empty_state.vue @@ -0,0 +1,93 @@ +<script> +import { mapActions, mapState, mapGetters } from 'vuex'; +import Icon from '~/vue_shared/components/icon.vue'; +import tooltip from '~/vue_shared/directives/tooltip'; + +export default { + components: { + Icon, + }, + directives: { + tooltip, + }, + props: { + noChangesStateSvgPath: { + type: String, + required: true, + }, + committedStateSvgPath: { + type: String, + required: true, + }, + }, + computed: { + ...mapState(['lastCommitMsg', 'rightPanelCollapsed']), + ...mapGetters(['collapseButtonIcon', 'collapseButtonTooltip']), + statusSvg() { + return this.lastCommitMsg ? this.committedStateSvgPath : this.noChangesStateSvgPath; + }, + }, + methods: { + ...mapActions(['toggleRightPanelCollapsed']), + }, +}; +</script> + +<template> + <div + class="multi-file-commit-panel-section ide-commit-empty-state js-empty-state" + > + <header + class="multi-file-commit-panel-header" + :class="{ + 'is-collapsed': rightPanelCollapsed, + }" + > + <button + v-tooltip + :title="collapseButtonTooltip" + data-container="body" + data-placement="left" + type="button" + class="btn btn-transparent multi-file-commit-panel-collapse-btn" + :aria-label="__('Toggle sidebar')" + @click.stop="toggleRightPanelCollapsed" + > + <icon + :name="collapseButtonIcon" + :size="18" + /> + </button> + </header> + <div + class="ide-commit-empty-state-container" + v-if="!rightPanelCollapsed" + > + <div class="svg-content svg-80"> + <img :src="statusSvg" /> + </div> + <div class="append-right-default prepend-left-default"> + <div + class="text-content text-center" + v-if="!lastCommitMsg" + > + <h4> + {{ __('No changes') }} + </h4> + <p> + {{ __('Edit files in the editor and commit changes here') }} + </p> + </div> + <div + class="text-content text-center" + v-else + > + <h4> + {{ __('All changes are committed') }} + </h4> + <p v-html="lastCommitMsg"></p> + </div> + </div> + </div> + </div> +</template> diff --git a/app/assets/javascripts/ide/components/commit_sidebar/list.vue b/app/assets/javascripts/ide/components/commit_sidebar/list.vue index 453208f3f19..ff05ee8682a 100644 --- a/app/assets/javascripts/ide/components/commit_sidebar/list.vue +++ b/app/assets/javascripts/ide/components/commit_sidebar/list.vue @@ -1,56 +1,132 @@ <script> - import { mapState } from 'vuex'; - import icon from '~/vue_shared/components/icon.vue'; - import listItem from './list_item.vue'; - import listCollapsed from './list_collapsed.vue'; +import { mapActions, mapState, mapGetters } from 'vuex'; +import { __, sprintf } from '~/locale'; +import Icon from '~/vue_shared/components/icon.vue'; +import tooltip from '~/vue_shared/directives/tooltip'; +import ListItem from './list_item.vue'; +import ListCollapsed from './list_collapsed.vue'; - export default { - components: { - icon, - listItem, - listCollapsed, +export default { + components: { + Icon, + ListItem, + ListCollapsed, + }, + directives: { + tooltip, + }, + props: { + title: { + type: String, + required: true, }, - props: { - title: { - type: String, - required: true, - }, - fileList: { - type: Array, - required: true, - }, + fileList: { + type: Array, + required: true, }, - computed: { - ...mapState([ - 'currentProjectId', - 'currentBranchId', - 'rightPanelCollapsed', - ]), - isCommitInfoShown() { - return this.rightPanelCollapsed || this.fileList.length; - }, + showToggle: { + type: Boolean, + required: false, + default: true, }, - methods: { - toggleCollapsed() { - this.$emit('toggleCollapsed'); - }, + iconName: { + type: String, + required: true, }, - }; + action: { + type: String, + required: true, + }, + actionBtnText: { + type: String, + required: true, + }, + itemActionComponent: { + type: String, + required: true, + }, + stagedList: { + type: Boolean, + required: false, + default: false, + }, + }, + computed: { + ...mapState(['rightPanelCollapsed']), + ...mapGetters(['collapseButtonIcon', 'collapseButtonTooltip']), + titleText() { + return sprintf(__('%{title} changes'), { + title: this.title, + }); + }, + }, + methods: { + ...mapActions(['toggleRightPanelCollapsed', 'stageAllChanges', 'unstageAllChanges']), + actionBtnClicked() { + this[this.action](); + }, + }, +}; </script> <template> <div + class="ide-commit-list-container" :class="{ - 'multi-file-commit-list': isCommitInfoShown + 'is-collapsed': rightPanelCollapsed, }" > + <header + class="multi-file-commit-panel-header" + > + <div + v-if="!rightPanelCollapsed" + class="multi-file-commit-panel-header-title" + :class="{ + 'append-right-10': showToggle, + }" + > + <icon + v-once + :name="iconName" + :size="18" + /> + {{ titleText }} + <button + type="button" + class="btn btn-blank btn-link ide-staged-action-btn" + @click="actionBtnClicked" + > + {{ actionBtnText }} + </button> + </div> + <button + v-if="showToggle" + v-tooltip + :title="collapseButtonTooltip" + data-container="body" + data-placement="left" + type="button" + class="btn btn-transparent multi-file-commit-panel-collapse-btn" + :aria-label="__('Toggle sidebar')" + @click.stop="toggleRightPanelCollapsed" + > + <icon + :name="collapseButtonIcon" + :size="18" + /> + </button> + </header> <list-collapsed v-if="rightPanelCollapsed" + :files="fileList" + :icon-name="iconName" + :title="title" /> <template v-else> <ul v-if="fileList.length" - class="list-unstyled append-bottom-0" + class="multi-file-commit-list list-unstyled append-bottom-0" > <li v-for="file in fileList" @@ -58,9 +134,18 @@ > <list-item :file="file" + :action-component="itemActionComponent" + :key-prefix="title" + :staged-list="stagedList" /> </li> </ul> + <p + v-else + class="multi-file-commit-list help-block" + > + {{ __('No changes') }} + </p> </template> </div> </template> diff --git a/app/assets/javascripts/ide/components/commit_sidebar/list_collapsed.vue b/app/assets/javascripts/ide/components/commit_sidebar/list_collapsed.vue index 15918ac9631..2254271c679 100644 --- a/app/assets/javascripts/ide/components/commit_sidebar/list_collapsed.vue +++ b/app/assets/javascripts/ide/components/commit_sidebar/list_collapsed.vue @@ -1,35 +1,110 @@ <script> - import { mapGetters } from 'vuex'; - import icon from '~/vue_shared/components/icon.vue'; +import Icon from '~/vue_shared/components/icon.vue'; +import tooltip from '~/vue_shared/directives/tooltip'; +import { sprintf, n__, __ } from '~/locale'; - export default { - components: { - icon, +export default { + components: { + Icon, + }, + directives: { + tooltip, + }, + props: { + files: { + type: Array, + required: true, }, - computed: { - ...mapGetters([ - 'addedFiles', - 'modifiedFiles', - ]), + iconName: { + type: String, + required: true, }, - }; + title: { + type: String, + required: true, + }, + }, + computed: { + addedFilesLength() { + return this.files.filter(f => f.tempFile).length; + }, + modifiedFilesLength() { + return this.files.filter(f => !f.tempFile).length; + }, + addedFilesIconClass() { + return this.addedFilesLength ? 'multi-file-addition' : ''; + }, + modifiedFilesClass() { + return this.modifiedFilesLength ? 'multi-file-modified' : ''; + }, + additionsTooltip() { + return sprintf(n__('1 %{type} addition', '%d %{type} additions', this.addedFilesLength), { + type: this.title.toLowerCase(), + }); + }, + modifiedTooltip() { + return sprintf( + n__('1 %{type} modification', '%d %{type} modifications', this.modifiedFilesLength), + { type: this.title.toLowerCase() }, + ); + }, + titleTooltip() { + return sprintf(__('%{title} changes'), { title: this.title }); + }, + additionIconName() { + return this.title.toLowerCase() === 'staged' ? 'file-addition-solid' : 'file-addition'; + }, + modifiedIconName() { + return this.title.toLowerCase() === 'staged' ? 'file-modified-solid' : 'file-modified'; + }, + }, +}; </script> <template> <div class="multi-file-commit-list-collapsed text-center" > - <icon - name="file-addition" - :size="18" - css-classes="multi-file-addition append-bottom-10" - /> - {{ addedFiles.length }} - <icon - name="file-modified" - :size="18" - css-classes="multi-file-modified prepend-top-10 append-bottom-10" - /> - {{ modifiedFiles.length }} + <div + v-tooltip + :title="titleTooltip" + data-container="body" + data-placement="left" + class="append-bottom-15" + > + <icon + v-once + :name="iconName" + :size="18" + /> + </div> + <div + v-tooltip + :title="additionsTooltip" + data-container="body" + data-placement="left" + class="append-bottom-10" + > + <icon + :name="additionIconName" + :size="18" + :css-classes="addedFilesIconClass" + /> + </div> + {{ addedFilesLength }} + <div + v-tooltip + :title="modifiedTooltip" + data-container="body" + data-placement="left" + class="prepend-top-10 append-bottom-10" + > + <icon + :name="modifiedIconName" + :size="18" + :css-classes="modifiedFilesClass" + /> + </div> + {{ modifiedFilesLength }} </div> </template> diff --git a/app/assets/javascripts/ide/components/commit_sidebar/list_item.vue b/app/assets/javascripts/ide/components/commit_sidebar/list_item.vue index 560cdd941cd..ad4713c40d5 100644 --- a/app/assets/javascripts/ide/components/commit_sidebar/list_item.vue +++ b/app/assets/javascripts/ide/components/commit_sidebar/list_item.vue @@ -1,34 +1,69 @@ <script> import { mapActions } from 'vuex'; import Icon from '~/vue_shared/components/icon.vue'; +import StageButton from './stage_button.vue'; +import UnstageButton from './unstage_button.vue'; export default { components: { Icon, + StageButton, + UnstageButton, }, props: { file: { type: Object, required: true, }, + actionComponent: { + type: String, + required: true, + }, + keyPrefix: { + type: String, + required: false, + default: '', + }, + stagedList: { + type: Boolean, + required: false, + default: false, + }, }, computed: { iconName() { - return this.file.tempFile ? 'file-addition' : 'file-modified'; + const prefix = this.stagedList ? '-solid' : ''; + return this.file.tempFile ? `file-addition${prefix}` : `file-modified${prefix}`; }, iconClass() { - return `multi-file-${this.file.tempFile ? 'addition' : 'modified'} append-right-8`; + return `multi-file-${this.file.tempFile ? 'additions' : 'modified'} append-right-8`; }, }, methods: { - ...mapActions(['discardFileChanges', 'updateViewer', 'openPendingTab']), - openFileInEditor(file) { - return this.openPendingTab(file).then(changeViewer => { + ...mapActions([ + 'discardFileChanges', + 'updateViewer', + 'openPendingTab', + 'unstageChange', + 'stageChange', + ]), + openFileInEditor() { + return this.openPendingTab({ + file: this.file, + keyPrefix: this.keyPrefix.toLowerCase(), + }).then(changeViewer => { if (changeViewer) { this.updateViewer('diff'); } }); }, + fileAction() { + if (this.file.staged) { + this.unstageChange(this.file.path); + } else { + this.stageChange(this.file.path); + } + }, }, }; </script> @@ -38,7 +73,9 @@ export default { <button type="button" class="multi-file-commit-list-path" - @click="openFileInEditor(file)"> + @dblclick="fileAction" + @click="openFileInEditor" + > <span class="multi-file-commit-list-file-path"> <icon :name="iconName" @@ -47,12 +84,9 @@ export default { />{{ file.path }} </span> </button> - <button - type="button" - class="btn btn-blank multi-file-discard-btn" - @click="discardFileChanges(file.path)" - > - Discard - </button> + <component + :is="actionComponent" + :path="file.path" + /> </div> </template> diff --git a/app/assets/javascripts/ide/components/commit_sidebar/stage_button.vue b/app/assets/javascripts/ide/components/commit_sidebar/stage_button.vue new file mode 100644 index 00000000000..52dce8412ab --- /dev/null +++ b/app/assets/javascripts/ide/components/commit_sidebar/stage_button.vue @@ -0,0 +1,59 @@ +<script> +import { mapActions } from 'vuex'; +import Icon from '~/vue_shared/components/icon.vue'; +import tooltip from '~/vue_shared/directives/tooltip'; + +export default { + components: { + Icon, + }, + directives: { + tooltip, + }, + props: { + path: { + type: String, + required: true, + }, + }, + methods: { + ...mapActions(['stageChange', 'discardFileChanges']), + }, +}; +</script> + +<template> + <div + v-once + class="multi-file-discard-btn" + > + <button + v-tooltip + type="button" + class="btn btn-blank append-right-5" + :aria-label="__('Stage changes')" + :title="__('Stage changes')" + data-container="body" + @click.stop="stageChange(path)" + > + <icon + name="mobile-issue-close" + :size="12" + /> + </button> + <button + v-tooltip + type="button" + class="btn btn-blank" + :aria-label="__('Discard changes')" + :title="__('Discard changes')" + data-container="body" + @click.stop="discardFileChanges(path)" + > + <icon + name="remove" + :size="12" + /> + </button> + </div> +</template> diff --git a/app/assets/javascripts/ide/components/commit_sidebar/unstage_button.vue b/app/assets/javascripts/ide/components/commit_sidebar/unstage_button.vue new file mode 100644 index 00000000000..123d60da47e --- /dev/null +++ b/app/assets/javascripts/ide/components/commit_sidebar/unstage_button.vue @@ -0,0 +1,45 @@ +<script> +import { mapActions } from 'vuex'; +import Icon from '~/vue_shared/components/icon.vue'; +import tooltip from '~/vue_shared/directives/tooltip'; + +export default { + components: { + Icon, + }, + directives: { + tooltip, + }, + props: { + path: { + type: String, + required: true, + }, + }, + methods: { + ...mapActions(['unstageChange']), + }, +}; +</script> + +<template> + <div + v-once + class="multi-file-discard-btn" + > + <button + v-tooltip + type="button" + class="btn btn-blank" + :aria-label="__('Unstage changes')" + :title="__('Unstage changes')" + data-container="body" + @click="unstageChange(path)" + > + <icon + name="history" + :size="12" + /> + </button> + </div> +</template> diff --git a/app/assets/javascripts/ide/components/ide_context_bar.vue b/app/assets/javascripts/ide/components/ide_context_bar.vue index 79a83b47994..627fbeb9adf 100644 --- a/app/assets/javascripts/ide/components/ide_context_bar.vue +++ b/app/assets/javascripts/ide/components/ide_context_bar.vue @@ -1,5 +1,4 @@ <script> -import { mapActions, mapGetters, mapState } from 'vuex'; import icon from '~/vue_shared/components/icon.vue'; import panelResizer from '~/vue_shared/components/panel_resizer.vue'; import repoCommitSection from './repo_commit_section.vue'; @@ -22,13 +21,6 @@ export default { required: true, }, }, - computed: { - ...mapState(['changedFiles', 'rightPanelCollapsed']), - ...mapGetters(['currentIcon']), - }, - methods: { - ...mapActions(['setPanelCollapsedStatus']), - }, }; </script> @@ -41,40 +33,6 @@ export default { <div class="multi-file-commit-panel-section" > - <header - class="multi-file-commit-panel-header" - :class="{ - 'is-collapsed': rightPanelCollapsed, - }" - > - <div - class="multi-file-commit-panel-header-title" - v-if="!rightPanelCollapsed" - > - <div - v-if="changedFiles.length" - > - <icon - name="list-bulleted" - :size="18" - /> - Staged - </div> - </div> - <button - type="button" - class="btn btn-transparent multi-file-commit-panel-collapse-btn" - @click.stop="setPanelCollapsedStatus({ - side: 'right', - collapsed: !rightPanelCollapsed, - })" - > - <icon - :name="currentIcon" - :size="18" - /> - </button> - </header> <repo-commit-section :no-changes-state-svg-path="noChangesStateSvgPath" :committed-state-svg-path="committedStateSvgPath" diff --git a/app/assets/javascripts/ide/components/ide_status_bar.vue b/app/assets/javascripts/ide/components/ide_status_bar.vue index 152a5f632ad..c13eeeace3f 100644 --- a/app/assets/javascripts/ide/components/ide_status_bar.vue +++ b/app/assets/javascripts/ide/components/ide_status_bar.vue @@ -22,13 +22,6 @@ export default { <template> <div class="ide-status-bar"> - <div class="ref-name"> - <icon - name="branch" - :size="12" - /> - {{ file.branchId }} - </div> <div> <div v-if="file.lastCommit && file.lastCommit.id"> Last commit: diff --git a/app/assets/javascripts/ide/components/repo_commit_section.vue b/app/assets/javascripts/ide/components/repo_commit_section.vue index 14673754503..877d1b5e026 100644 --- a/app/assets/javascripts/ide/components/repo_commit_section.vue +++ b/app/assets/javascripts/ide/components/repo_commit_section.vue @@ -1,10 +1,11 @@ <script> import { mapState, mapActions, mapGetters } from 'vuex'; import tooltip from '~/vue_shared/directives/tooltip'; -import icon from '~/vue_shared/components/icon.vue'; +import Icon from '~/vue_shared/components/icon.vue'; import DeprecatedModal from '~/vue_shared/components/deprecated_modal.vue'; import LoadingButton from '~/vue_shared/components/loading_button.vue'; -import commitFilesList from './commit_sidebar/list.vue'; +import CommitFilesList from './commit_sidebar/list.vue'; +import EmptyState from './commit_sidebar/empty_state.vue'; import CommitMessageField from './commit_sidebar/message_field.vue'; import * as consts from '../stores/modules/commit/constants'; import Actions from './commit_sidebar/actions.vue'; @@ -12,8 +13,9 @@ import Actions from './commit_sidebar/actions.vue'; export default { components: { DeprecatedModal, - icon, - commitFilesList, + Icon, + CommitFilesList, + EmptyState, Actions, LoadingButton, CommitMessageField, @@ -32,33 +34,17 @@ export default { }, }, computed: { - ...mapState([ - 'currentProjectId', - 'currentBranchId', - 'rightPanelCollapsed', - 'lastCommitMsg', - 'changedFiles', - ]), + ...mapState(['changedFiles', 'stagedFiles', 'rightPanelCollapsed']), ...mapState('commit', ['commitMessage', 'submitCommitLoading']), ...mapGetters('commit', ['commitButtonDisabled', 'discardDraftButtonDisabled', 'branchName']), - statusSvg() { - return this.lastCommitMsg ? this.committedStateSvgPath : this.noChangesStateSvgPath; - }, }, methods: { - ...mapActions(['setPanelCollapsedStatus']), ...mapActions('commit', [ 'updateCommitMessage', 'discardDraft', 'commitChanges', 'updateCommitAction', ]), - toggleCollapsed() { - this.setPanelCollapsedStatus({ - side: 'right', - collapsed: !this.rightPanelCollapsed, - }); - }, forceCreateNewBranch() { return this.updateCommitAction(consts.COMMIT_TO_NEW_BRANCH).then(() => this.commitChanges()); }, @@ -69,9 +55,6 @@ export default { <template> <div class="multi-file-commit-panel-section" - :class="{ - 'multi-file-commit-empty-state-container': !changedFiles.length - }" > <deprecated-modal id="ide-create-branch-modal" @@ -85,15 +68,27 @@ export default { Would you like to create a new branch?`) }} </template> </deprecated-modal> - <commit-files-list - title="Staged" - :file-list="changedFiles" - :collapsed="rightPanelCollapsed" - @toggleCollapsed="toggleCollapsed" - /> <template - v-if="changedFiles.length" + v-if="changedFiles.length || stagedFiles.length" > + <commit-files-list + icon-name="unstaged" + :title="__('Unstaged')" + :file-list="changedFiles" + action="stageAllChanges" + :action-btn-text="__('Stage all')" + item-action-component="stage-button" + /> + <commit-files-list + icon-name="staged" + :title="__('Staged')" + :file-list="stagedFiles" + action="unstageAllChanges" + :action-btn-text="__('Unstage all')" + item-action-component="unstage-button" + :show-toggle="false" + :staged-list="true" + /> <form class="form-horizontal multi-file-commit-form" @submit.prevent.stop="commitChanges" @@ -123,38 +118,10 @@ export default { </div> </form> </template> - <div - v-else-if="!rightPanelCollapsed" - class="row js-empty-state" - > - <div class="col-xs-10 col-xs-offset-1"> - <div class="svg-content svg-80"> - <img :src="statusSvg" /> - </div> - </div> - <div class="col-xs-10 col-xs-offset-1"> - <div - class="text-content text-center" - v-if="!lastCommitMsg" - > - <h4> - {{ __('No changes') }} - </h4> - <p> - {{ __('Edit files in the editor and commit changes here') }} - </p> - </div> - <div - class="text-content text-center" - v-else - > - <h4> - {{ __('All changes are committed') }} - </h4> - <p v-html="lastCommitMsg"> - </p> - </div> - </div> - </div> + <empty-state + v-else + :no-changes-state-svg-path="noChangesStateSvgPath" + :committed-state-svg-path="committedStateSvgPath" + /> </div> </template> diff --git a/app/assets/javascripts/ide/components/repo_editor.vue b/app/assets/javascripts/ide/components/repo_editor.vue index 711bafa17a9..3a04cdd8e46 100644 --- a/app/assets/javascripts/ide/components/repo_editor.vue +++ b/app/assets/javascripts/ide/components/repo_editor.vue @@ -20,7 +20,7 @@ export default { }, computed: { ...mapState(['rightPanelCollapsed', 'viewer', 'delayViewerUpdated', 'panelResizing']), - ...mapGetters(['currentMergeRequest']), + ...mapGetters(['currentMergeRequest', 'getStagedFile']), shouldHideEditor() { return this.file && this.file.binary && !this.file.content; }, @@ -120,7 +120,12 @@ export default { setupEditor() { if (!this.file || !this.editor.instance) return; - this.model = this.editor.createModel(this.file); + const head = this.getStagedFile(this.file.path); + + this.model = this.editor.createModel( + this.file, + this.file.staged && this.file.key.indexOf('unstaged-') === 0 ? head : null, + ); if (this.viewer === 'mrdiff') { this.editor.attachMergeRequestModel(this.model); diff --git a/app/assets/javascripts/ide/components/repo_file.vue b/app/assets/javascripts/ide/components/repo_file.vue index 3b5068d4910..8b18c7d28b4 100644 --- a/app/assets/javascripts/ide/components/repo_file.vue +++ b/app/assets/javascripts/ide/components/repo_file.vue @@ -102,8 +102,11 @@ export default { v-if="file.mrChange" /> <changed-file-icon + v-if="file.changed || file.tempFile || file.staged" :file="file" - v-if="file.changed || file.tempFile" + :show-tooltip="true" + :show-staged-icon="true" + class="prepend-top-5 pull-right" /> </span> <new-dropdown diff --git a/app/assets/javascripts/ide/components/repo_tab.vue b/app/assets/javascripts/ide/components/repo_tab.vue index 304a73ed1ad..35a362b01e0 100644 --- a/app/assets/javascripts/ide/components/repo_tab.vue +++ b/app/assets/javascripts/ide/components/repo_tab.vue @@ -26,13 +26,16 @@ export default { }, computed: { closeLabel() { - if (this.tab.changed || this.tab.tempFile) { + if (this.fileHasChanged) { return `${this.tab.name} changed`; } return `Close ${this.tab.name}`; }, showChangedIcon() { - return this.tab.changed ? !this.tabMouseOver : false; + return this.fileHasChanged ? !this.tabMouseOver : false; + }, + fileHasChanged() { + return this.tab.changed || this.tab.tempFile || this.tab.staged; }, }, @@ -42,18 +45,18 @@ export default { this.updateDelayViewerUpdated(true); if (tab.pending) { - this.openPendingTab(tab); + this.openPendingTab({ file: tab, keyPrefix: tab.staged ? 'staged' : 'unstaged' }); } else { this.$router.push(`/project${tab.url}`); } }, mouseOverTab() { - if (this.tab.changed) { + if (this.fileHasChanged) { this.tabMouseOver = true; } }, mouseOutTab() { - if (this.tab.changed) { + if (this.fileHasChanged) { this.tabMouseOver = false; } }, diff --git a/app/assets/javascripts/ide/lib/common/model.js b/app/assets/javascripts/ide/lib/common/model.js index e47adae99ed..d47b6704176 100644 --- a/app/assets/javascripts/ide/lib/common/model.js +++ b/app/assets/javascripts/ide/lib/common/model.js @@ -3,15 +3,16 @@ import Disposable from './disposable'; import eventHub from '../../eventhub'; export default class Model { - constructor(monaco, file) { + constructor(monaco, file, head = null) { this.monaco = monaco; this.disposable = new Disposable(); this.file = file; + this.head = head; this.content = file.content !== '' ? file.content : file.raw; this.disposable.add( (this.originalModel = this.monaco.editor.createModel( - this.file.raw, + head ? head.content : this.file.raw, undefined, new this.monaco.Uri(null, null, `original/${this.file.key}`), )), @@ -34,10 +35,12 @@ export default class Model { this.events = new Map(); this.updateContent = this.updateContent.bind(this); + this.updateNewContent = this.updateNewContent.bind(this); this.dispose = this.dispose.bind(this); eventHub.$on(`editor.update.model.dispose.${this.file.key}`, this.dispose); - eventHub.$on(`editor.update.model.content.${this.file.path}`, this.updateContent); + eventHub.$on(`editor.update.model.content.${this.file.key}`, this.updateContent); + eventHub.$on(`editor.update.model.new.content.${this.file.key}`, this.updateNewContent); } get url() { @@ -79,8 +82,15 @@ export default class Model { ); } - updateContent(content) { + updateContent({ content, changed }) { this.getOriginalModel().setValue(content); + + if (!changed) { + this.getModel().setValue(content); + } + } + + updateNewContent(content) { this.getModel().setValue(content); } @@ -89,6 +99,7 @@ export default class Model { this.events.clear(); eventHub.$off(`editor.update.model.dispose.${this.file.key}`, this.dispose); - eventHub.$off(`editor.update.model.content.${this.file.path}`, this.updateContent); + eventHub.$off(`editor.update.model.content.${this.file.key}`, this.updateContent); + eventHub.$off(`editor.update.model.new.content.${this.file.key}`, this.updateNewContent); } } diff --git a/app/assets/javascripts/ide/lib/common/model_manager.js b/app/assets/javascripts/ide/lib/common/model_manager.js index 0e7b563b5d6..7f643969480 100644 --- a/app/assets/javascripts/ide/lib/common/model_manager.js +++ b/app/assets/javascripts/ide/lib/common/model_manager.js @@ -17,12 +17,12 @@ export default class ModelManager { return this.models.get(key); } - addModel(file) { + addModel(file, head = null) { if (this.hasCachedModel(file.key)) { return this.getModel(file.key); } - const model = new Model(this.monaco, file); + const model = new Model(this.monaco, file, head); this.models.set(model.path, model); this.disposable.add(model); diff --git a/app/assets/javascripts/ide/lib/editor.js b/app/assets/javascripts/ide/lib/editor.js index d537cc1fbae..b65d9c68a0b 100644 --- a/app/assets/javascripts/ide/lib/editor.js +++ b/app/assets/javascripts/ide/lib/editor.js @@ -83,8 +83,8 @@ export default class Editor { } } - createModel(file) { - return this.modelManager.addModel(file); + createModel(file, head = null) { + return this.modelManager.addModel(file, head); } attachModel(model) { diff --git a/app/assets/javascripts/ide/stores/actions.js b/app/assets/javascripts/ide/stores/actions.js index 57420b6c699..7158e94b6c3 100644 --- a/app/assets/javascripts/ide/stores/actions.js +++ b/app/assets/javascripts/ide/stores/actions.js @@ -1,3 +1,4 @@ +import $ from 'jquery'; import Vue from 'vue'; import { visitUrl } from '~/lib/utils/url_utility'; import flash from '~/flash'; @@ -32,6 +33,22 @@ export const setPanelCollapsedStatus = ({ commit }, { side, collapsed }) => { } }; +export const toggleRightPanelCollapsed = ( + { dispatch, state }, + e = undefined, +) => { + if (e) { + $(e.currentTarget) + .tooltip('hide') + .blur(); + } + + dispatch('setPanelCollapsedStatus', { + side: 'right', + collapsed: !state.rightPanelCollapsed, + }); +}; + export const setResizingStatus = ({ commit }, resizing) => { commit(types.SET_RESIZING_STATUS, resizing); }; @@ -104,6 +121,14 @@ export const scrollToTab = () => { }); }; +export const stageAllChanges = ({ state, commit }) => { + state.changedFiles.forEach(file => commit(types.STAGE_CHANGE, file.path)); +}; + +export const unstageAllChanges = ({ state, commit }) => { + state.stagedFiles.forEach(file => commit(types.UNSTAGE_CHANGE, file.path)); +}; + export const updateViewer = ({ commit }, viewer) => { commit(types.UPDATE_VIEWER, viewer); }; diff --git a/app/assets/javascripts/ide/stores/actions/file.js b/app/assets/javascripts/ide/stores/actions/file.js index 1a17320a1ea..d782e0a84d2 100644 --- a/app/assets/javascripts/ide/stores/actions/file.js +++ b/app/assets/javascripts/ide/stores/actions/file.js @@ -24,7 +24,10 @@ export const closeFile = ({ commit, state, dispatch }, file) => { if (nextFileToOpen.pending) { dispatch('updateViewer', 'diff'); - dispatch('openPendingTab', nextFileToOpen); + dispatch('openPendingTab', { + file: nextFileToOpen, + keyPrefix: nextFileToOpen.staged ? 'staged' : 'unstaged', + }); } else { dispatch('updateDelayViewerUpdated', true); router.push(`/project${nextFileToOpen.url}`); @@ -153,7 +156,7 @@ export const setFileViewMode = ({ state, commit }, { file, viewMode }) => { commit(types.SET_FILE_VIEWMODE, { file, viewMode }); }; -export const discardFileChanges = ({ state, commit }, path) => { +export const discardFileChanges = ({ dispatch, state, commit, getters }, path) => { const file = state.entries[path]; commit(types.DISCARD_FILE_CHANGES, path); @@ -161,17 +164,40 @@ export const discardFileChanges = ({ state, commit }, path) => { if (file.tempFile && file.opened) { commit(types.TOGGLE_FILE_OPEN, path); + } else if (getters.activeFile && file.path === getters.activeFile.path) { + dispatch('updateDelayViewerUpdated', true) + .then(() => { + router.push(`/project${file.url}`); + }) + .catch(e => { + throw e; + }); + } + + eventHub.$emit(`editor.update.model.new.content.${file.key}`, file.content); + eventHub.$emit(`editor.update.model.dispose.unstaged-${file.key}`, file.content); +}; + +export const stageChange = ({ commit, state }, path) => { + const stagedFile = state.stagedFiles.find(f => f.path === path); + + commit(types.STAGE_CHANGE, path); + + if (stagedFile) { + eventHub.$emit(`editor.update.model.new.content.staged-${stagedFile.key}`, stagedFile.content); } +}; - eventHub.$emit(`editor.update.model.content.${file.path}`, file.raw); +export const unstageChange = ({ commit }, path) => { + commit(types.UNSTAGE_CHANGE, path); }; -export const openPendingTab = ({ commit, getters, dispatch, state }, file) => { - if (getters.activeFile && getters.activeFile.path === file.path && state.viewer === 'diff') { +export const openPendingTab = ({ commit, getters, dispatch, state }, { file, keyPrefix }) => { + if (getters.activeFile && getters.activeFile === file && state.viewer === 'diff') { return false; } - commit(types.ADD_PENDING_TAB, { file }); + commit(types.ADD_PENDING_TAB, { file, keyPrefix }); dispatch('scrollToTab'); diff --git a/app/assets/javascripts/ide/stores/getters.js b/app/assets/javascripts/ide/stores/getters.js index 14658c98d0b..535c4d6f902 100644 --- a/app/assets/javascripts/ide/stores/getters.js +++ b/app/assets/javascripts/ide/stores/getters.js @@ -1,3 +1,5 @@ +import { __ } from '~/locale'; + export const activeFile = state => state.openFiles.find(file => file.active) || null; export const addedFiles = state => state.changedFiles.filter(f => f.tempFile); @@ -29,10 +31,14 @@ export const currentMergeRequest = state => { }; // eslint-disable-next-line no-confusing-arrow -export const currentIcon = state => +export const collapseButtonIcon = state => state.rightPanelCollapsed ? 'angle-double-left' : 'angle-double-right'; -export const hasChanges = state => !!state.changedFiles.length; +export const hasChanges = state => !!state.changedFiles.length || !!state.stagedFiles.length; + +// eslint-disable-next-line no-confusing-arrow +export const collapseButtonTooltip = state => + state.rightPanelCollapsed ? __('Expand sidebar') : __('Collapse sidebar'); export const hasMergeRequest = state => !!state.currentMergeRequestId; @@ -48,3 +54,5 @@ export const allBlobs = state => return acc; }, []) .sort((a, b) => b.lastOpenedAt - a.lastOpenedAt); + +export const getStagedFile = state => path => state.stagedFiles.find(f => f.path === path); diff --git a/app/assets/javascripts/ide/stores/modules/commit/actions.js b/app/assets/javascripts/ide/stores/modules/commit/actions.js index 367c45f7e2d..b26512e213a 100644 --- a/app/assets/javascripts/ide/stores/modules/commit/actions.js +++ b/app/assets/javascripts/ide/stores/modules/commit/actions.js @@ -98,40 +98,25 @@ export const updateFilesAfterCommit = ( { root: true }, ); - rootState.changedFiles.forEach(entry => { - commit( - rootTypes.SET_LAST_COMMIT_DATA, - { - entry, - lastCommit, - }, - { root: true }, - ); - - eventHub.$emit(`editor.update.model.content.${entry.path}`, entry.content); + rootState.stagedFiles.forEach(file => { + const changedFile = rootState.changedFiles.find(f => f.path === file.path); commit( - rootTypes.SET_FILE_RAW_DATA, + rootTypes.UPDATE_FILE_AFTER_COMMIT, { - file: entry, - raw: entry.content, + file, + lastCommit, }, { root: true }, ); - commit( - rootTypes.TOGGLE_FILE_CHANGED, - { - file: entry, - changed: false, - }, - { root: true }, - ); + eventHub.$emit(`editor.update.model.content.${file.key}`, { + content: file.content, + changed: !!changedFile, + }); }); - commit(rootTypes.REMOVE_ALL_CHANGES_FILES, null, { root: true }); - - if (state.commitAction === consts.COMMIT_TO_NEW_BRANCH) { + if (state.commitAction === consts.COMMIT_TO_NEW_BRANCH && rootGetters.activeFile) { router.push( `/project/${rootState.currentProjectId}/blob/${branch}/${rootGetters.activeFile.path}`, ); @@ -184,6 +169,8 @@ export const commitChanges = ({ commit, state, getters, dispatch, rootState }) = { root: true }, ); } + + commit(rootTypes.CLEAR_STAGED_CHANGES, null, { root: true }); }) .then(() => dispatch('updateCommitAction', consts.COMMIT_TO_CURRENT_BRANCH)); }) diff --git a/app/assets/javascripts/ide/stores/modules/commit/getters.js b/app/assets/javascripts/ide/stores/modules/commit/getters.js index f7cdd6adb0c..9c3905a0b0d 100644 --- a/app/assets/javascripts/ide/stores/modules/commit/getters.js +++ b/app/assets/javascripts/ide/stores/modules/commit/getters.js @@ -1,12 +1,17 @@ import * as consts from './constants'; -export const discardDraftButtonDisabled = state => state.commitMessage === '' || state.submitCommitLoading; +const BRANCH_SUFFIX_COUNT = 5; + +export const discardDraftButtonDisabled = state => + state.commitMessage === '' || state.submitCommitLoading; export const commitButtonDisabled = (state, getters, rootState) => - getters.discardDraftButtonDisabled || !rootState.changedFiles.length; + getters.discardDraftButtonDisabled || !rootState.stagedFiles.length; export const newBranchName = (state, _, rootState) => - `${gon.current_username}-${rootState.currentBranchId}-patch-${`${new Date().getTime()}`.substr(-5)}`; + `${gon.current_username}-${rootState.currentBranchId}-patch-${`${new Date().getTime()}`.substr( + -BRANCH_SUFFIX_COUNT, + )}`; export const branchName = (state, getters, rootState) => { if ( diff --git a/app/assets/javascripts/ide/stores/mutation_types.js b/app/assets/javascripts/ide/stores/mutation_types.js index 742bd40f25d..c7f08449d03 100644 --- a/app/assets/javascripts/ide/stores/mutation_types.js +++ b/app/assets/javascripts/ide/stores/mutation_types.js @@ -51,6 +51,11 @@ export const SET_FILE_MERGE_REQUEST_CHANGE = 'SET_FILE_MERGE_REQUEST_CHANGE'; export const UPDATE_VIEWER = 'UPDATE_VIEWER'; export const UPDATE_DELAY_VIEWER_CHANGE = 'UPDATE_DELAY_VIEWER_CHANGE'; +export const CLEAR_STAGED_CHANGES = 'CLEAR_STAGED_CHANGES'; +export const STAGE_CHANGE = 'STAGE_CHANGE'; +export const UNSTAGE_CHANGE = 'UNSTAGE_CHANGE'; + +export const UPDATE_FILE_AFTER_COMMIT = 'UPDATE_FILE_AFTER_COMMIT'; export const ADD_PENDING_TAB = 'ADD_PENDING_TAB'; export const REMOVE_PENDING_TAB = 'REMOVE_PENDING_TAB'; diff --git a/app/assets/javascripts/ide/stores/mutations.js b/app/assets/javascripts/ide/stores/mutations.js index 4510ef64262..2a6c136aeed 100644 --- a/app/assets/javascripts/ide/stores/mutations.js +++ b/app/assets/javascripts/ide/stores/mutations.js @@ -49,6 +49,11 @@ export default { lastCommitMsg, }); }, + [types.CLEAR_STAGED_CHANGES](state) { + Object.assign(state, { + stagedFiles: [], + }); + }, [types.SET_ENTRIES](state, entries) { Object.assign(state, { entries, @@ -100,6 +105,22 @@ export default { fileFindVisible, }); }, + [types.UPDATE_FILE_AFTER_COMMIT](state, { file, lastCommit }) { + const changedFile = state.changedFiles.find(f => f.path === file.path); + + Object.assign(state.entries[file.path], { + raw: file.content, + changed: !!changedFile, + staged: false, + lastCommit: Object.assign(state.entries[file.path].lastCommit, { + id: lastCommit.commit.id, + url: lastCommit.commit_path, + message: lastCommit.commit.message, + author: lastCommit.commit.author_name, + updatedAt: lastCommit.commit.authored_date, + }), + }); + }, ...projectMutations, ...mergeRequestMutation, ...fileMutations, diff --git a/app/assets/javascripts/ide/stores/mutations/file.js b/app/assets/javascripts/ide/stores/mutations/file.js index 66ca6745d14..c3041c77199 100644 --- a/app/assets/javascripts/ide/stores/mutations/file.js +++ b/app/assets/javascripts/ide/stores/mutations/file.js @@ -58,7 +58,9 @@ export default { }); }, [types.UPDATE_FILE_CONTENT](state, { path, content }) { - const changed = content !== state.entries[path].raw; + const stagedFile = state.stagedFiles.find(f => f.path === path); + const rawContent = stagedFile ? stagedFile.content : state.entries[path].raw; + const changed = content !== rawContent; Object.assign(state.entries[path], { content, @@ -92,8 +94,10 @@ export default { }); }, [types.DISCARD_FILE_CHANGES](state, path) { + const stagedFile = state.stagedFiles.find(f => f.path === path); + Object.assign(state.entries[path], { - content: state.entries[path].raw, + content: stagedFile ? stagedFile.content : state.entries[path].raw, changed: false, }); }, @@ -107,16 +111,67 @@ export default { changedFiles: state.changedFiles.filter(f => f.path !== path), }); }, + [types.STAGE_CHANGE](state, path) { + const stagedFile = state.stagedFiles.find(f => f.path === path); + + Object.assign(state, { + changedFiles: state.changedFiles.filter(f => f.path !== path), + entries: Object.assign(state.entries, { + [path]: Object.assign(state.entries[path], { + staged: true, + changed: false, + }), + }), + }); + + if (stagedFile) { + Object.assign(stagedFile, { + ...state.entries[path], + }); + } else { + Object.assign(state, { + stagedFiles: state.stagedFiles.concat({ + ...state.entries[path], + }), + }); + } + }, + [types.UNSTAGE_CHANGE](state, path) { + const changedFile = state.changedFiles.find(f => f.path === path); + const stagedFile = state.stagedFiles.find(f => f.path === path); + + if (!changedFile && stagedFile) { + Object.assign(state.entries[path], { + ...stagedFile, + key: state.entries[path].key, + active: state.entries[path].active, + opened: state.entries[path].opened, + changed: true, + }); + + Object.assign(state, { + changedFiles: state.changedFiles.concat(state.entries[path]), + }); + } + + Object.assign(state, { + stagedFiles: state.stagedFiles.filter(f => f.path !== path), + entries: Object.assign(state.entries, { + [path]: Object.assign(state.entries[path], { + staged: false, + }), + }), + }); + }, [types.TOGGLE_FILE_CHANGED](state, { file, changed }) { Object.assign(state.entries[file.path], { changed, }); }, [types.ADD_PENDING_TAB](state, { file, keyPrefix = 'pending' }) { - const pendingTab = state.openFiles.find(f => f.path === file.path && f.pending); - let openFiles = state.openFiles.map(f => - Object.assign(f, { active: f.path === file.path, opened: false }), - ); + const key = `${keyPrefix}-${file.key}`; + const pendingTab = state.openFiles.find(f => f.key === key && f.pending); + let openFiles = state.openFiles.map(f => Object.assign(f, { active: false, opened: false })); if (!pendingTab) { const openFile = openFiles.find(f => f.path === file.path); @@ -127,10 +182,11 @@ export default { if (f.path === file.path) { return acc.concat({ ...f, + content: file.content, active: true, pending: true, opened: true, - key: `${keyPrefix}-${f.key}`, + key, }); } diff --git a/app/assets/javascripts/ide/stores/state.js b/app/assets/javascripts/ide/stores/state.js index 3694ad953fa..3470bb9aec0 100644 --- a/app/assets/javascripts/ide/stores/state.js +++ b/app/assets/javascripts/ide/stores/state.js @@ -3,6 +3,7 @@ export default () => ({ currentBranchId: '', currentMergeRequestId: '', changedFiles: [], + stagedFiles: [], endpoints: {}, lastCommitMsg: '', lastCommitPath: '', diff --git a/app/assets/javascripts/ide/stores/utils.js b/app/assets/javascripts/ide/stores/utils.js index 6fa7369b1fa..86612d845e0 100644 --- a/app/assets/javascripts/ide/stores/utils.js +++ b/app/assets/javascripts/ide/stores/utils.js @@ -15,6 +15,7 @@ export const dataStructure = () => ({ opened: false, active: false, changed: false, + staged: false, lastCommitPath: '', lastCommit: { id: '', @@ -102,7 +103,7 @@ export const setPageTitle = title => { export const createCommitPayload = (branch, newBranch, state, rootState) => ({ branch, commit_message: state.commitMessage, - actions: rootState.changedFiles.map(f => ({ + actions: rootState.stagedFiles.map(f => ({ action: f.tempFile ? 'create' : 'update', file_path: f.path, content: f.content, diff --git a/app/assets/stylesheets/pages/repo.scss b/app/assets/stylesheets/pages/repo.scss index 5381bdc2105..6d77260b658 100644 --- a/app/assets/stylesheets/pages/repo.scss +++ b/app/assets/stylesheets/pages/repo.scss @@ -69,6 +69,10 @@ .ide-file-changed-icon { margin-left: auto; + + > svg { + display: block; + } } .ide-new-btn { @@ -379,7 +383,11 @@ padding: $gl-bar-padding $gl-padding; background: $white-light; display: flex; - justify-content: space-between; + justify-content: flex-end; + + > div + div { + padding-left: $gl-padding; + } svg { vertical-align: middle; @@ -522,9 +530,13 @@ overflow: auto; } -.multi-file-commit-empty-state-container { - align-items: center; - justify-content: center; +.ide-commit-empty-state { + padding: 0 $gl-padding; +} + +.ide-commit-empty-state-container { + margin-top: auto; + margin-bottom: auto; } .multi-file-commit-panel-header { @@ -533,35 +545,22 @@ margin-bottom: 0; border-bottom: 1px solid $white-dark; padding: $gl-btn-padding 0; - - &.is-collapsed { - border-bottom: 1px solid $white-dark; - - svg { - margin-left: auto; - margin-right: auto; - } - - .multi-file-commit-panel-collapse-btn { - margin-right: auto; - margin-left: auto; - border-left: 0; - } - } } .multi-file-commit-panel-header-title { display: flex; flex: 1; - padding: 0 $gl-btn-padding; + padding-left: $grid-size; svg { margin-right: $gl-btn-padding; + color: $theme-gray-700; } } .multi-file-commit-panel-collapse-btn { border-left: 1px solid $white-dark; + margin-left: auto; } .multi-file-commit-list { @@ -575,12 +574,14 @@ display: flex; padding: 0; align-items: center; + border-radius: $border-radius-default; .multi-file-discard-btn { display: none; + margin-top: -2px; margin-left: auto; + margin-right: $grid-size; color: $gl-link-color; - padding: 0 2px; &:focus, &:hover { @@ -592,26 +593,31 @@ background: $white-normal; .multi-file-discard-btn { - display: block; + display: flex; } } } -.multi-file-addition { +.multi-file-additions, +.multi-file-additions-solid { fill: $green-500; } -.multi-file-modified { +.multi-file-modified, +.multi-file-modified-solid { fill: $orange-500; } .multi-file-commit-list-collapsed { display: flex; flex-direction: column; + padding: $gl-padding 0; - > svg { + svg { + display: block; margin-left: auto; margin-right: auto; + color: $theme-gray-700; } .file-status-icon { @@ -623,7 +629,7 @@ .multi-file-commit-list-path { padding: $grid-size / 2; - padding-left: $gl-padding; + padding-left: $grid-size; background: none; border: 0; text-align: left; @@ -808,6 +814,41 @@ } } +.ide-commit-list-container { + display: flex; + flex-direction: column; + width: 100%; + padding: 0 16px; + + &:not(.is-collapsed) { + flex: 1; + min-height: 140px; + } + + &.is-collapsed { + .multi-file-commit-panel-header { + margin-left: -$gl-padding; + margin-right: -$gl-padding; + + svg { + margin-left: auto; + margin-right: auto; + } + + .multi-file-commit-panel-collapse-btn { + margin-right: auto; + margin-left: auto; + border-left: 0; + } + } + } +} + +.ide-staged-action-btn { + margin-left: auto; + color: $gl-link-color; +} + .ide-commit-radios { label { font-weight: normal; diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index c7dc0826738..b0c02cdeec7 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -20,7 +20,7 @@ module Ci has_one :last_deployment, -> { order('deployments.id DESC') }, as: :deployable, class_name: 'Deployment' has_many :trace_sections, class_name: 'Ci::BuildTraceSection' - has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :job_id, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :job_id, dependent: :destroy, inverse_of: :job # rubocop:disable Cop/ActiveRecordDependent has_one :job_artifacts_archive, -> { where(file_type: Ci::JobArtifact.file_types[:archive]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id has_one :job_artifacts_metadata, -> { where(file_type: Ci::JobArtifact.file_types[:metadata]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id has_one :job_artifacts_trace, -> { where(file_type: Ci::JobArtifact.file_types[:trace]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id @@ -95,8 +95,8 @@ module Ci run_after_commit { BuildHooksWorker.perform_async(build.id) } end - after_commit :update_project_statistics_after_save, on: [:create, :update] - after_commit :update_project_statistics, on: :destroy + after_save :update_project_statistics_after_save, if: :artifacts_size_changed? + after_destroy :update_project_statistics_after_destroy, unless: :project_destroyed? class << self # This is needed for url_for to work, @@ -664,16 +664,20 @@ module Ci pipeline.config_processor.build_attributes(name) end - def update_project_statistics - return unless project + def update_project_statistics_after_save + update_project_statistics(read_attribute(:artifacts_size).to_i - artifacts_size_was.to_i) + end - ProjectCacheWorker.perform_async(project_id, [], [:build_artifacts_size]) + def update_project_statistics_after_destroy + update_project_statistics(-artifacts_size) end - def update_project_statistics_after_save - if previous_changes.include?('artifacts_size') - update_project_statistics - end + def update_project_statistics(difference) + ProjectStatistics.increment_statistic(project_id, :build_artifacts_size, difference) + end + + def project_destroyed? + project.pending_delete? end end end diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index fbb95fe16df..39676efa08c 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -7,12 +7,15 @@ module Ci belongs_to :project belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id - before_save :update_file_store + mount_uploader :file, JobArtifactUploader + before_save :set_size, if: :file_changed? + after_save :update_project_statistics_after_save, if: :size_changed? + after_destroy :update_project_statistics_after_destroy, unless: :project_destroyed? - scope :with_files_stored_locally, -> { where(file_store: [nil, ::JobArtifactUploader::Store::LOCAL]) } + after_save :update_file_store - mount_uploader :file, JobArtifactUploader + scope :with_files_stored_locally, -> { where(file_store: [nil, ::JobArtifactUploader::Store::LOCAL]) } delegate :exists?, :open, to: :file @@ -23,7 +26,9 @@ module Ci } def update_file_store - self.file_store = file.object_store + # The file.object_store is set during `uploader.store!` + # which happens after object is inserted/updated + self.update_column(:file_store, file.object_store) end def self.artifacts_size_for(project) @@ -34,10 +39,6 @@ module Ci [nil, ::JobArtifactUploader::Store::LOCAL].include?(self.file_store) end - def set_size - self.size = file.size - end - def expire_in expire_at - Time.now if expire_at end @@ -48,5 +49,28 @@ module Ci ChronicDuration.parse(value)&.seconds&.from_now end end + + private + + def set_size + self.size = file.size + end + + def update_project_statistics_after_save + update_project_statistics(size.to_i - size_was.to_i) + end + + def update_project_statistics_after_destroy + update_project_statistics(-self.size) + end + + def update_project_statistics(difference) + ProjectStatistics.increment_statistic(project_id, :build_artifacts_size, difference) + end + + def project_destroyed? + # Use job.project to avoid extra DB query for project + job.project.pending_delete? + end end end diff --git a/app/models/lfs_object.rb b/app/models/lfs_object.rb index b7de46fa202..6b7f280fb70 100644 --- a/app/models/lfs_object.rb +++ b/app/models/lfs_object.rb @@ -11,10 +11,12 @@ class LfsObject < ActiveRecord::Base mount_uploader :file, LfsObjectUploader - before_save :update_file_store + after_save :update_file_store def update_file_store - self.file_store = file.object_store + # The file.object_store is set during `uploader.store!` + # which happens after object is inserted/updated + self.update_column(:file_store, file.object_store) end def project_allowed_access?(project) diff --git a/app/models/project_statistics.rb b/app/models/project_statistics.rb index 87a4350f022..5d4e3c34b39 100644 --- a/app/models/project_statistics.rb +++ b/app/models/project_statistics.rb @@ -4,15 +4,15 @@ class ProjectStatistics < ActiveRecord::Base before_save :update_storage_size - STORAGE_COLUMNS = [:repository_size, :lfs_objects_size, :build_artifacts_size].freeze - STATISTICS_COLUMNS = [:commit_count] + STORAGE_COLUMNS + COLUMNS_TO_REFRESH = [:repository_size, :lfs_objects_size, :commit_count].freeze + INCREMENTABLE_COLUMNS = [:build_artifacts_size].freeze def total_repository_size repository_size + lfs_objects_size end def refresh!(only: nil) - STATISTICS_COLUMNS.each do |column, generator| + COLUMNS_TO_REFRESH.each do |column, generator| if only.blank? || only.include?(column) public_send("update_#{column}") # rubocop:disable GitlabSecurity/PublicSend end @@ -34,13 +34,15 @@ class ProjectStatistics < ActiveRecord::Base self.lfs_objects_size = project.lfs_objects.sum(:size) end - def update_build_artifacts_size - self.build_artifacts_size = - project.builds.sum(:artifacts_size) + - Ci::JobArtifact.artifacts_size_for(self.project) + def update_storage_size + self.storage_size = repository_size + lfs_objects_size + build_artifacts_size end - def update_storage_size - self.storage_size = STORAGE_COLUMNS.sum(&method(:read_attribute)) + def self.increment_statistic(project_id, key, amount) + raise ArgumentError, "Cannot increment attribute: #{key}" unless key.in?(INCREMENTABLE_COLUMNS) + return if amount == 0 + + where(project_id: project_id) + .update_all(["#{key} = COALESCE(#{key}, 0) + (?)", amount]) end end diff --git a/app/services/labels/transfer_service.rb b/app/services/labels/transfer_service.rb index 775efed48eb..9b7486cf53b 100644 --- a/app/services/labels/transfer_service.rb +++ b/app/services/labels/transfer_service.rb @@ -64,9 +64,14 @@ module Labels end def update_label_links(labels, old_label_id:, new_label_id:) - LabelLink.joins(:label) - .merge(labels) - .where(label_id: old_label_id) + # use 'labels' relation to get label_link ids only of issues/MRs + # in the project being transferred. + # IDs are fetched in a separate query because MySQL doesn't + # allow referring of 'label_links' table in UPDATE query: + # https://gitlab.com/gitlab-org/gitlab-ce/-/jobs/62435068 + link_ids = labels.pluck('label_links.id') + + LabelLink.where(id: link_ids, label_id: old_label_id) .update_all(label_id: new_label_id) end diff --git a/app/uploaders/job_artifact_uploader.rb b/app/uploaders/job_artifact_uploader.rb index dd86753479d..2a5a830ce4f 100644 --- a/app/uploaders/job_artifact_uploader.rb +++ b/app/uploaders/job_artifact_uploader.rb @@ -6,10 +6,10 @@ class JobArtifactUploader < GitlabUploader storage_options Gitlab.config.artifacts - def size - return super if model.size.nil? + def cached_size + return model.size if model.size.present? && !model.file_changed? - model.size + size end def store_dir @@ -20,7 +20,7 @@ class JobArtifactUploader < GitlabUploader if file_storage? File.open(path, "rb") if path else - ::Gitlab::Ci::Trace::HttpIO.new(url, size) if url + ::Gitlab::Ci::Trace::HttpIO.new(url, cached_size) if url end end diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index bd258e04d3f..a3549cada95 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -183,14 +183,6 @@ module ObjectStorage StoreURL: connection.put_object_url(remote_store_path, upload_path, expire_at, options) } end - - def default_object_store - if self.object_store_enabled? && self.direct_upload_enabled? - Store::REMOTE - else - Store::LOCAL - end - end end # allow to configure and overwrite the filename @@ -211,12 +203,13 @@ module ObjectStorage end def object_store - @object_store ||= model.try(store_serialization_column) || self.class.default_object_store + # We use Store::LOCAL as null value indicates the local storage + @object_store ||= model.try(store_serialization_column) || Store::LOCAL end # rubocop:disable Gitlab/ModuleWithInstanceVariables def object_store=(value) - @object_store = value || self.class.default_object_store + @object_store = value || Store::LOCAL @storage = storage_for(object_store) end # rubocop:enable Gitlab/ModuleWithInstanceVariables @@ -302,6 +295,15 @@ module ObjectStorage super end + def store!(new_file = nil) + # when direct upload is enabled, always store on remote storage + if self.class.object_store_enabled? && self.class.direct_upload_enabled? + self.object_store = Store::REMOTE + end + + super + end + private def schedule_background_upload? diff --git a/changelogs/unreleased/41059-calculate-artifact-size-more-efficiently.yml b/changelogs/unreleased/41059-calculate-artifact-size-more-efficiently.yml new file mode 100644 index 00000000000..e3f94bbf081 --- /dev/null +++ b/changelogs/unreleased/41059-calculate-artifact-size-more-efficiently.yml @@ -0,0 +1,5 @@ +--- +title: Improve DB performance of calculating total artifacts size +merge_request: 17839 +author: +type: performance diff --git a/changelogs/unreleased/44834-ide-remove-branch-from-bottom-bar.yml b/changelogs/unreleased/44834-ide-remove-branch-from-bottom-bar.yml new file mode 100644 index 00000000000..d3f838ad362 --- /dev/null +++ b/changelogs/unreleased/44834-ide-remove-branch-from-bottom-bar.yml @@ -0,0 +1,5 @@ +--- +title: Remove branch name from the status bar of WebIDE +merge_request: +author: +type: changed diff --git a/changelogs/unreleased/fix-direct-upload-for-old-records.yml b/changelogs/unreleased/fix-direct-upload-for-old-records.yml new file mode 100644 index 00000000000..a062b9e73e9 --- /dev/null +++ b/changelogs/unreleased/fix-direct-upload-for-old-records.yml @@ -0,0 +1,5 @@ +--- +title: Fix direct_upload when records with null file_store are used +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/label-links-on-project-transfer.yml b/changelogs/unreleased/label-links-on-project-transfer.yml new file mode 100644 index 00000000000..fabedb77cb3 --- /dev/null +++ b/changelogs/unreleased/label-links-on-project-transfer.yml @@ -0,0 +1,5 @@ +--- +title: Fix label links update on project transfer +merge_request: +author: +type: fixed diff --git a/config/initializers/active_record_avoid_type_casting_in_uniqueness_validator.rb b/config/initializers/active_record_avoid_type_casting_in_uniqueness_validator.rb new file mode 100644 index 00000000000..d9418caf68b --- /dev/null +++ b/config/initializers/active_record_avoid_type_casting_in_uniqueness_validator.rb @@ -0,0 +1,98 @@ +# This is a monkey patch which must be removed when migrating to Rails 5.1 from 5.0. +# +# In Rails 5.0 there was introduced a bug which casts types in the uniqueness validator. +# https://github.com/rails/rails/pull/23523/commits/811a4fa8eb6ceea841e61e8ac05747ffb69595ae +# +# That causes to bugs like this: +# +# 1) API::Users POST /user/:id/gpg_keys/:key_id/revoke when authenticated revokes existing key +# Failure/Error: let(:gpg_key) { create(:gpg_key, user: user) } +# +# TypeError: +# can't cast Hash +# # ./spec/requests/api/users_spec.rb:7:in `block (2 levels) in <top (required)>' +# # ./spec/requests/api/users_spec.rb:908:in `block (4 levels) in <top (required)>' +# # ------------------ +# # --- Caused by: --- +# # TypeError: +# # TypeError +# # ./spec/requests/api/users_spec.rb:7:in `block (2 levels) in <top (required)>' +# +# This bug was fixed in Rails 5.1 by https://github.com/rails/rails/pull/24745/commits/aa062318c451512035c10898a1af95943b1a3803 + +if Gitlab.rails5? + ActiveSupport::Deprecation.warn("#{__FILE__} is a monkey patch which must be removed when upgrading to Rails 5.1") + + if Rails.version.start_with?("5.1") + raise "Remove this monkey patch: #{__FILE__}" + end + + # Copy-paste from https://github.com/kamipo/rails/blob/aa062318c451512035c10898a1af95943b1a3803/activerecord/lib/active_record/validations/uniqueness.rb + # including local fixes to make Rubocop happy again. + module ActiveRecord + module Validations + class UniquenessValidator < ActiveModel::EachValidator # :nodoc: + def validate_each(record, attribute, value) + finder_class = find_finder_class_for(record) + table = finder_class.arel_table + value = map_enum_attribute(finder_class, attribute, value) + + relation = build_relation(finder_class, table, attribute, value) + + if record.persisted? + if finder_class.primary_key + relation = relation.where.not(finder_class.primary_key => record.id_was || record.id) + else + raise UnknownPrimaryKey.new(finder_class, "Can not validate uniqueness for persisted record without primary key.") + end + end + + relation = scope_relation(record, table, relation) + relation = relation.merge(options[:conditions]) if options[:conditions] + + if relation.exists? + error_options = options.except(:case_sensitive, :scope, :conditions) + error_options[:value] = value + + record.errors.add(attribute, :taken, error_options) + end + rescue RangeError + end + + protected + + def build_relation(klass, table, attribute, value) #:nodoc: + if reflection = klass._reflect_on_association(attribute) + attribute = reflection.foreign_key + value = value.attributes[reflection.klass.primary_key] unless value.nil? + end + + # the attribute may be an aliased attribute + if klass.attribute_alias?(attribute) + attribute = klass.attribute_alias(attribute) + end + + attribute_name = attribute.to_s + + column = klass.columns_hash[attribute_name] + cast_type = klass.type_for_attribute(attribute_name) + + comparison = + if !options[:case_sensitive] && !value.nil? + # will use SQL LOWER function before comparison, unless it detects a case insensitive collation + klass.connection.case_insensitive_comparison(table, attribute, column, value) + else + klass.connection.case_sensitive_comparison(table, attribute, column, value) + end + + if value.nil? + klass.unscoped.where(comparison) + else + bind = Relation::QueryAttribute.new(attribute_name, value, cast_type) + klass.unscoped.where(comparison, bind) + end + end + end + end + end +end diff --git a/spec/features/projects/tree/create_directory_spec.rb b/spec/features/projects/tree/create_directory_spec.rb index d96c7e655ba..b242e41df1c 100644 --- a/spec/features/projects/tree/create_directory_spec.rb +++ b/spec/features/projects/tree/create_directory_spec.rb @@ -44,6 +44,8 @@ feature 'Multi-file editor new directory', :js do wait_for_requests + click_button 'Stage all' + fill_in('commit-message', with: 'commit message ide') click_button('Commit') diff --git a/spec/features/projects/tree/create_file_spec.rb b/spec/features/projects/tree/create_file_spec.rb index a4cbd5cf766..7d65456e049 100644 --- a/spec/features/projects/tree/create_file_spec.rb +++ b/spec/features/projects/tree/create_file_spec.rb @@ -34,6 +34,8 @@ feature 'Multi-file editor new file', :js do wait_for_requests + click_button 'Stage all' + fill_in('commit-message', with: 'commit message ide') click_button('Commit') diff --git a/spec/javascripts/ide/components/commit_sidebar/empty_state_spec.js b/spec/javascripts/ide/components/commit_sidebar/empty_state_spec.js new file mode 100644 index 00000000000..b80d08de7b1 --- /dev/null +++ b/spec/javascripts/ide/components/commit_sidebar/empty_state_spec.js @@ -0,0 +1,95 @@ +import Vue from 'vue'; +import store from '~/ide/stores'; +import emptyState from '~/ide/components/commit_sidebar/empty_state.vue'; +import { createComponentWithStore } from '../../../helpers/vue_mount_component_helper'; +import { resetStore } from '../../helpers'; + +describe('IDE commit panel empty state', () => { + let vm; + + beforeEach(() => { + const Component = Vue.extend(emptyState); + + vm = createComponentWithStore(Component, store, { + noChangesStateSvgPath: 'no-changes', + committedStateSvgPath: 'committed-state', + }); + + vm.$mount(); + }); + + afterEach(() => { + vm.$destroy(); + + resetStore(vm.$store); + }); + + describe('statusSvg', () => { + it('uses noChangesStateSvgPath when commit message is empty', () => { + expect(vm.statusSvg).toBe('no-changes'); + expect(vm.$el.querySelector('img').getAttribute('src')).toBe( + 'no-changes', + ); + }); + + it('uses committedStateSvgPath when commit message exists', done => { + vm.$store.state.lastCommitMsg = 'testing'; + + Vue.nextTick(() => { + expect(vm.statusSvg).toBe('committed-state'); + expect(vm.$el.querySelector('img').getAttribute('src')).toBe( + 'committed-state', + ); + + done(); + }); + }); + }); + + it('renders no changes text when last commit message is empty', () => { + expect(vm.$el.textContent).toContain('No changes'); + }); + + it('renders last commit message when it exists', done => { + vm.$store.state.lastCommitMsg = 'testing commit message'; + + Vue.nextTick(() => { + expect(vm.$el.textContent).toContain('testing commit message'); + + done(); + }); + }); + + describe('toggle button', () => { + it('calls store action', () => { + spyOn(vm, 'toggleRightPanelCollapsed'); + + vm.$el.querySelector('.multi-file-commit-panel-collapse-btn').click(); + + expect(vm.toggleRightPanelCollapsed).toHaveBeenCalled(); + }); + + it('renders collapsed class', done => { + vm.$el.querySelector('.multi-file-commit-panel-collapse-btn').click(); + + Vue.nextTick(() => { + expect(vm.$el.querySelector('.is-collapsed')).not.toBeNull(); + + done(); + }); + }); + }); + + describe('collapsed state', () => { + beforeEach(done => { + vm.$store.state.rightPanelCollapsed = true; + + Vue.nextTick(done); + }); + + it('does not render text & svg', () => { + expect(vm.$el.querySelector('img')).toBeNull(); + expect(vm.$el.textContent).not.toContain('No changes'); + }); + }); +}); diff --git a/spec/javascripts/ide/components/commit_sidebar/list_collapsed_spec.js b/spec/javascripts/ide/components/commit_sidebar/list_collapsed_spec.js index 32dbc3bf72e..9af3c15a4e3 100644 --- a/spec/javascripts/ide/components/commit_sidebar/list_collapsed_spec.js +++ b/spec/javascripts/ide/components/commit_sidebar/list_collapsed_spec.js @@ -11,10 +11,17 @@ describe('Multi-file editor commit sidebar list collapsed', () => { beforeEach(() => { const Component = Vue.extend(listCollapsed); - vm = createComponentWithStore(Component, store); - - vm.$store.state.changedFiles.push(file('file1'), file('file2')); - vm.$store.state.changedFiles[0].tempFile = true; + vm = createComponentWithStore(Component, store, { + files: [ + { + ...file('file1'), + tempFile: true, + }, + file('file2'), + ], + iconName: 'staged', + title: 'Staged', + }); vm.$mount(); }); @@ -26,4 +33,40 @@ describe('Multi-file editor commit sidebar list collapsed', () => { it('renders added & modified files count', () => { expect(removeWhitespace(vm.$el.textContent).trim()).toBe('1 1'); }); + + describe('addedFilesLength', () => { + it('returns an length of temp files', () => { + expect(vm.addedFilesLength).toBe(1); + }); + }); + + describe('modifiedFilesLength', () => { + it('returns an length of modified files', () => { + expect(vm.modifiedFilesLength).toBe(1); + }); + }); + + describe('addedFilesIconClass', () => { + it('includes multi-file-addition when addedFiles is not empty', () => { + expect(vm.addedFilesIconClass).toContain('multi-file-addition'); + }); + + it('excludes multi-file-addition when addedFiles is empty', () => { + vm.files = []; + + expect(vm.addedFilesIconClass).not.toContain('multi-file-addition'); + }); + }); + + describe('modifiedFilesClass', () => { + it('includes multi-file-modified when addedFiles is not empty', () => { + expect(vm.modifiedFilesClass).toContain('multi-file-modified'); + }); + + it('excludes multi-file-modified when addedFiles is empty', () => { + vm.files = []; + + expect(vm.modifiedFilesClass).not.toContain('multi-file-modified'); + }); + }); }); diff --git a/spec/javascripts/ide/components/commit_sidebar/list_item_spec.js b/spec/javascripts/ide/components/commit_sidebar/list_item_spec.js index 509434e4300..cc7e0a3f26d 100644 --- a/spec/javascripts/ide/components/commit_sidebar/list_item_spec.js +++ b/spec/javascripts/ide/components/commit_sidebar/list_item_spec.js @@ -1,7 +1,7 @@ import Vue from 'vue'; +import store from '~/ide/stores'; import listItem from '~/ide/components/commit_sidebar/list_item.vue'; import router from '~/ide/ide_router'; -import store from '~/ide/stores'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; import { file, resetStore } from '../../helpers'; @@ -18,6 +18,7 @@ describe('Multi-file editor commit sidebar list item', () => { vm = createComponentWithStore(Component, store, { file: f, + actionComponent: 'stage-button', }).$mount(); }); @@ -31,22 +32,18 @@ describe('Multi-file editor commit sidebar list item', () => { expect(vm.$el.querySelector('.multi-file-commit-list-path').textContent.trim()).toBe(f.path); }); - it('calls discardFileChanges when clicking discard button', () => { - spyOn(vm, 'discardFileChanges'); - - vm.$el.querySelector('.multi-file-discard-btn').click(); - - expect(vm.discardFileChanges).toHaveBeenCalled(); + it('renders actionn button', () => { + expect(vm.$el.querySelector('.multi-file-discard-btn')).not.toBeNull(); }); it('opens a closed file in the editor when clicking the file path', done => { - spyOn(vm, 'openFileInEditor').and.callThrough(); + spyOn(vm, 'openPendingTab').and.callThrough(); spyOn(router, 'push'); vm.$el.querySelector('.multi-file-commit-list-path').click(); setTimeout(() => { - expect(vm.openFileInEditor).toHaveBeenCalled(); + expect(vm.openPendingTab).toHaveBeenCalled(); expect(router.push).toHaveBeenCalled(); done(); diff --git a/spec/javascripts/ide/components/commit_sidebar/list_spec.js b/spec/javascripts/ide/components/commit_sidebar/list_spec.js index a62c0a28340..62fc3f90ad1 100644 --- a/spec/javascripts/ide/components/commit_sidebar/list_spec.js +++ b/spec/javascripts/ide/components/commit_sidebar/list_spec.js @@ -2,7 +2,7 @@ import Vue from 'vue'; import store from '~/ide/stores'; import commitSidebarList from '~/ide/components/commit_sidebar/list.vue'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; -import { file } from '../../helpers'; +import { file, resetStore } from '../../helpers'; describe('Multi-file editor commit sidebar list', () => { let vm; @@ -13,6 +13,10 @@ describe('Multi-file editor commit sidebar list', () => { vm = createComponentWithStore(Component, store, { title: 'Staged', fileList: [], + iconName: 'staged', + action: 'stageAllChanges', + actionBtnText: 'stage all', + itemActionComponent: 'stage-button', }); vm.$store.state.rightPanelCollapsed = false; @@ -22,6 +26,8 @@ describe('Multi-file editor commit sidebar list', () => { afterEach(() => { vm.$destroy(); + + resetStore(vm.$store); }); describe('with a list of files', () => { @@ -38,6 +44,12 @@ describe('Multi-file editor commit sidebar list', () => { }); }); + describe('empty files array', () => { + it('renders no changes text when empty', () => { + expect(vm.$el.textContent).toContain('No changes'); + }); + }); + describe('collapsed', () => { beforeEach(done => { vm.$store.state.rightPanelCollapsed = true; @@ -50,4 +62,32 @@ describe('Multi-file editor commit sidebar list', () => { expect(vm.$el.querySelector('.help-block')).toBeNull(); }); }); + + describe('with toggle', () => { + beforeEach(done => { + spyOn(vm, 'toggleRightPanelCollapsed'); + + vm.showToggle = true; + + Vue.nextTick(done); + }); + + it('calls setPanelCollapsedStatus when clickin toggle', () => { + vm.$el.querySelector('.multi-file-commit-panel-collapse-btn').click(); + + expect(vm.toggleRightPanelCollapsed).toHaveBeenCalled(); + }); + }); + + describe('action button', () => { + beforeEach(() => { + spyOn(vm, 'stageAllChanges'); + }); + + it('calls store action when clicked', () => { + vm.$el.querySelector('.ide-staged-action-btn').click(); + + expect(vm.stageAllChanges).toHaveBeenCalled(); + }); + }); }); diff --git a/spec/javascripts/ide/components/commit_sidebar/stage_button_spec.js b/spec/javascripts/ide/components/commit_sidebar/stage_button_spec.js new file mode 100644 index 00000000000..6bf8710bda7 --- /dev/null +++ b/spec/javascripts/ide/components/commit_sidebar/stage_button_spec.js @@ -0,0 +1,46 @@ +import Vue from 'vue'; +import store from '~/ide/stores'; +import stageButton from '~/ide/components/commit_sidebar/stage_button.vue'; +import { createComponentWithStore } from '../../../helpers/vue_mount_component_helper'; +import { file, resetStore } from '../../helpers'; + +describe('IDE stage file button', () => { + let vm; + let f; + + beforeEach(() => { + const Component = Vue.extend(stageButton); + f = file(); + + vm = createComponentWithStore(Component, store, { + path: f.path, + }); + + spyOn(vm, 'stageChange'); + spyOn(vm, 'discardFileChanges'); + + vm.$mount(); + }); + + afterEach(() => { + vm.$destroy(); + + resetStore(vm.$store); + }); + + it('renders button to discard & stage', () => { + expect(vm.$el.querySelectorAll('.btn').length).toBe(2); + }); + + it('calls store with stage button', () => { + vm.$el.querySelectorAll('.btn')[0].click(); + + expect(vm.stageChange).toHaveBeenCalledWith(f.path); + }); + + it('calls store with discard button', () => { + vm.$el.querySelectorAll('.btn')[1].click(); + + expect(vm.discardFileChanges).toHaveBeenCalledWith(f.path); + }); +}); diff --git a/spec/javascripts/ide/components/commit_sidebar/unstage_button_spec.js b/spec/javascripts/ide/components/commit_sidebar/unstage_button_spec.js new file mode 100644 index 00000000000..917bbb9fb46 --- /dev/null +++ b/spec/javascripts/ide/components/commit_sidebar/unstage_button_spec.js @@ -0,0 +1,39 @@ +import Vue from 'vue'; +import store from '~/ide/stores'; +import unstageButton from '~/ide/components/commit_sidebar/unstage_button.vue'; +import { createComponentWithStore } from '../../../helpers/vue_mount_component_helper'; +import { file, resetStore } from '../../helpers'; + +describe('IDE unstage file button', () => { + let vm; + let f; + + beforeEach(() => { + const Component = Vue.extend(unstageButton); + f = file(); + + vm = createComponentWithStore(Component, store, { + path: f.path, + }); + + spyOn(vm, 'unstageChange'); + + vm.$mount(); + }); + + afterEach(() => { + vm.$destroy(); + + resetStore(vm.$store); + }); + + it('renders button to unstage', () => { + expect(vm.$el.querySelectorAll('.btn').length).toBe(1); + }); + + it('calls store with unnstage button', () => { + vm.$el.querySelector('.btn').click(); + + expect(vm.unstageChange).toHaveBeenCalledWith(f.path); + }); +}); diff --git a/spec/javascripts/ide/components/repo_commit_section_spec.js b/spec/javascripts/ide/components/repo_commit_section_spec.js index 113ade269e9..768f6e99bf1 100644 --- a/spec/javascripts/ide/components/repo_commit_section_spec.js +++ b/spec/javascripts/ide/components/repo_commit_section_spec.js @@ -28,16 +28,34 @@ describe('RepoCommitSection', () => { }, }; + const files = [file('file1'), file('file2')].map(f => + Object.assign(f, { + type: 'blob', + }), + ); + vm.$store.state.rightPanelCollapsed = false; vm.$store.state.currentBranch = 'master'; - vm.$store.state.changedFiles = [file('file1'), file('file2')]; + vm.$store.state.changedFiles = [...files]; vm.$store.state.changedFiles.forEach(f => Object.assign(f, { changed: true, + content: 'changedFile testing', + }), + ); + + vm.$store.state.stagedFiles = [{ ...files[0] }, { ...files[1] }]; + vm.$store.state.stagedFiles.forEach(f => + Object.assign(f, { + changed: true, content: 'testing', }), ); + vm.$store.state.changedFiles.forEach(f => { + vm.$store.state.entries[f.path] = f; + }); + return vm.$mount(); } @@ -94,20 +112,93 @@ describe('RepoCommitSection', () => { ...vm.$el.querySelectorAll('.multi-file-commit-list li'), ]; const submitCommit = vm.$el.querySelector('form .btn'); + const allFiles = vm.$store.state.changedFiles.concat( + vm.$store.state.stagedFiles, + ); expect(vm.$el.querySelector('.multi-file-commit-form')).not.toBeNull(); - expect(changedFileElements.length).toEqual(2); + expect(changedFileElements.length).toEqual(4); changedFileElements.forEach((changedFile, i) => { - expect(changedFile.textContent.trim()).toContain( - vm.$store.state.changedFiles[i].path, - ); + expect(changedFile.textContent.trim()).toContain(allFiles[i].path); }); expect(submitCommit.disabled).toBeTruthy(); expect(submitCommit.querySelector('.fa-spinner.fa-spin')).toBeNull(); }); + it('adds changed files into staged files', done => { + vm.$el.querySelector('.ide-staged-action-btn').click(); + + Vue.nextTick(() => { + expect( + vm.$el.querySelector('.ide-commit-list-container').textContent, + ).toContain('No changes'); + + done(); + }); + }); + + it('stages a single file', done => { + vm.$el.querySelector('.multi-file-discard-btn .btn').click(); + + Vue.nextTick(() => { + expect( + vm.$el + .querySelector('.ide-commit-list-container') + .querySelectorAll('li').length, + ).toBe(1); + + done(); + }); + }); + + it('discards a single file', done => { + vm.$el.querySelectorAll('.multi-file-discard-btn .btn')[1].click(); + + Vue.nextTick(() => { + expect( + vm.$el.querySelector('.ide-commit-list-container').textContent, + ).not.toContain('file1'); + expect( + vm.$el + .querySelector('.ide-commit-list-container') + .querySelectorAll('li').length, + ).toBe(1); + + done(); + }); + }); + + it('removes all staged files', done => { + vm.$el.querySelectorAll('.ide-staged-action-btn')[1].click(); + + Vue.nextTick(() => { + expect( + vm.$el.querySelectorAll('.ide-commit-list-container')[1].textContent, + ).toContain('No changes'); + + done(); + }); + }); + + it('unstages a single file', done => { + vm.$el + .querySelectorAll('.multi-file-discard-btn')[2] + .querySelector('.btn') + .click(); + + Vue.nextTick(() => { + expect( + vm.$el + .querySelectorAll('.ide-commit-list-container')[1] + .querySelectorAll('li').length, + ).toBe(1); + + done(); + }); + }); + it('updates commitMessage in store on input', done => { const textarea = vm.$el.querySelector('textarea'); diff --git a/spec/javascripts/ide/components/repo_editor_spec.js b/spec/javascripts/ide/components/repo_editor_spec.js index 310d222377f..fc585647fbd 100644 --- a/spec/javascripts/ide/components/repo_editor_spec.js +++ b/spec/javascripts/ide/components/repo_editor_spec.js @@ -200,7 +200,7 @@ describe('RepoEditor', () => { vm.setupEditor(); - expect(vm.editor.createModel).toHaveBeenCalledWith(vm.file); + expect(vm.editor.createModel).toHaveBeenCalledWith(vm.file, null); expect(vm.model).not.toBeNull(); }); @@ -234,6 +234,20 @@ describe('RepoEditor', () => { done(); }); }); + + it('sets head model as staged file', () => { + spyOn(vm.editor, 'createModel').and.callThrough(); + + Editor.editorInstance.modelManager.dispose(); + + vm.$store.state.stagedFiles.push({ ...vm.file, key: 'staged' }); + vm.file.staged = true; + vm.file.key = `unstaged-${vm.file.key}`; + + vm.setupEditor(); + + expect(vm.editor.createModel).toHaveBeenCalledWith(vm.file, vm.$store.state.stagedFiles[0]); + }); }); describe('editor updateDimensions', () => { diff --git a/spec/javascripts/ide/lib/common/model_spec.js b/spec/javascripts/ide/lib/common/model_spec.js index 8fc2fccb64c..553a7a3746b 100644 --- a/spec/javascripts/ide/lib/common/model_spec.js +++ b/spec/javascripts/ide/lib/common/model_spec.js @@ -30,6 +30,19 @@ describe('Multi-file editor library model', () => { expect(model.baseModel).not.toBeNull(); }); + it('creates model with head file to compare against', () => { + const f = file('path'); + model.dispose(); + + model = new Model(monaco, f, { + ...f, + content: '123 testing', + }); + + expect(model.head).not.toBeNull(); + expect(model.getOriginalModel().getValue()).toBe('123 testing'); + }); + it('adds eventHub listener', () => { expect(eventHub.$on).toHaveBeenCalledWith( `editor.update.model.dispose.${model.file.key}`, diff --git a/spec/javascripts/ide/lib/editor_spec.js b/spec/javascripts/ide/lib/editor_spec.js index 75e6f0f54ec..530bdfa2759 100644 --- a/spec/javascripts/ide/lib/editor_spec.js +++ b/spec/javascripts/ide/lib/editor_spec.js @@ -88,7 +88,7 @@ describe('Multi-file editor library', () => { instance.createModel('FILE'); - expect(instance.modelManager.addModel).toHaveBeenCalledWith('FILE'); + expect(instance.modelManager.addModel).toHaveBeenCalledWith('FILE', null); }); }); diff --git a/spec/javascripts/ide/stores/actions/file_spec.js b/spec/javascripts/ide/stores/actions/file_spec.js index 479ed7ce49e..ce5c525bed7 100644 --- a/spec/javascripts/ide/stores/actions/file_spec.js +++ b/spec/javascripts/ide/stores/actions/file_spec.js @@ -1,9 +1,12 @@ import Vue from 'vue'; import store from '~/ide/stores'; +import * as actions from '~/ide/stores/actions/file'; +import * as types from '~/ide/stores/mutation_types'; import service from '~/ide/services'; import router from '~/ide/ide_router'; import eventHub from '~/ide/eventhub'; import { file, resetStore } from '../../helpers'; +import testAction from '../../../helpers/vuex_action_helper'; describe('IDE store file actions', () => { beforeEach(() => { @@ -402,6 +405,7 @@ describe('IDE store file actions', () => { beforeEach(() => { spyOn(eventHub, '$on'); + spyOn(eventHub, '$emit'); tmpFile = file(); tmpFile.content = 'testing'; @@ -460,6 +464,57 @@ describe('IDE store file actions', () => { }) .catch(done.fail); }); + + it('pushes route for active file', done => { + tmpFile.active = true; + store.state.openFiles.push(tmpFile); + + store + .dispatch('discardFileChanges', tmpFile.path) + .then(() => { + expect(router.push).toHaveBeenCalledWith(`/project${tmpFile.url}`); + + done(); + }) + .catch(done.fail); + }); + + it('emits eventHub event to dispose cached model', done => { + store + .dispatch('discardFileChanges', tmpFile.path) + .then(() => { + expect(eventHub.$emit).toHaveBeenCalled(); + + done(); + }) + .catch(done.fail); + }); + }); + + describe('stageChange', () => { + it('calls STAGE_CHANGE with file path', done => { + testAction( + actions.stageChange, + 'path', + store.state, + [{ type: types.STAGE_CHANGE, payload: 'path' }], + [], + done, + ); + }); + }); + + describe('unstageChange', () => { + it('calls UNSTAGE_CHANGE with file path', done => { + testAction( + actions.unstageChange, + 'path', + store.state, + [{ type: types.UNSTAGE_CHANGE, payload: 'path' }], + [], + done, + ); + }); }); describe('openPendingTab', () => { @@ -476,7 +531,7 @@ describe('IDE store file actions', () => { it('makes file pending in openFiles', done => { store - .dispatch('openPendingTab', f) + .dispatch('openPendingTab', { file: f, keyPrefix: 'pending' }) .then(() => { expect(store.state.openFiles[0].pending).toBe(true); }) @@ -486,7 +541,7 @@ describe('IDE store file actions', () => { it('returns true when opened', done => { store - .dispatch('openPendingTab', f) + .dispatch('openPendingTab', { file: f, keyPrefix: 'pending' }) .then(added => { expect(added).toBe(true); }) @@ -498,7 +553,7 @@ describe('IDE store file actions', () => { store.state.currentBranchId = 'master'; store - .dispatch('openPendingTab', f) + .dispatch('openPendingTab', { file: f, keyPrefix: 'pending' }) .then(() => { expect(router.push).toHaveBeenCalledWith('/project/123/tree/master/'); }) @@ -512,7 +567,7 @@ describe('IDE store file actions', () => { store._actions.scrollToTab = [scrollToTabSpy]; // eslint-disable-line store - .dispatch('openPendingTab', f) + .dispatch('openPendingTab', { file: f, keyPrefix: 'pending' }) .then(() => { expect(scrollToTabSpy).toHaveBeenCalled(); store._actions.scrollToTab = oldScrollToTab; // eslint-disable-line @@ -527,7 +582,7 @@ describe('IDE store file actions', () => { store.state.viewer = 'diff'; store - .dispatch('openPendingTab', f) + .dispatch('openPendingTab', { file: f, keyPrefix: 'pending' }) .then(added => { expect(added).toBe(false); }) diff --git a/spec/javascripts/ide/stores/actions_spec.js b/spec/javascripts/ide/stores/actions_spec.js index 87471f7a92f..f848f13d429 100644 --- a/spec/javascripts/ide/stores/actions_spec.js +++ b/spec/javascripts/ide/stores/actions_spec.js @@ -1,6 +1,7 @@ import * as urlUtils from '~/lib/utils/url_utility'; import store from '~/ide/stores'; import * as actions from '~/ide/stores/actions'; +import * as types from '~/ide/stores/mutation_types'; import router from '~/ide/ide_router'; import { resetStore, file } from '../helpers'; import testAction from '../../helpers/vuex_action_helper'; @@ -292,6 +293,42 @@ describe('Multi-file store actions', () => { }); }); + describe('stageAllChanges', () => { + it('adds all files from changedFiles to stagedFiles', done => { + store.state.changedFiles.push(file(), file('new')); + + testAction( + actions.stageAllChanges, + null, + store.state, + [ + { type: types.STAGE_CHANGE, payload: store.state.changedFiles[0].path }, + { type: types.STAGE_CHANGE, payload: store.state.changedFiles[1].path }, + ], + [], + done, + ); + }); + }); + + describe('unstageAllChanges', () => { + it('removes all files from stagedFiles after unstaging', done => { + store.state.stagedFiles.push(file(), file('new')); + + testAction( + actions.unstageAllChanges, + null, + store.state, + [ + { type: types.UNSTAGE_CHANGE, payload: store.state.stagedFiles[0].path }, + { type: types.UNSTAGE_CHANGE, payload: store.state.stagedFiles[1].path }, + ], + [], + done, + ); + }); + }); + describe('updateViewer', () => { it('updates viewer state', done => { store diff --git a/spec/javascripts/ide/stores/getters_spec.js b/spec/javascripts/ide/stores/getters_spec.js index 4a7d9a05402..b6b4dd28729 100644 --- a/spec/javascripts/ide/stores/getters_spec.js +++ b/spec/javascripts/ide/stores/getters_spec.js @@ -37,19 +37,11 @@ describe('IDE store getters', () => { expect(modifiedFiles.length).toBe(1); expect(modifiedFiles[0].name).toBe('changed'); }); - }); - describe('addedFiles', () => { - it('returns a list of added files', () => { - localState.openFiles.push(file()); - localState.changedFiles.push(file('added')); - localState.changedFiles[0].changed = true; - localState.changedFiles[0].tempFile = true; + it('returns angle left when collapsed', () => { + localState.rightPanelCollapsed = true; - const modifiedFiles = getters.addedFiles(localState); - - expect(modifiedFiles.length).toBe(1); - expect(modifiedFiles[0].name).toBe('added'); + expect(getters.collapseButtonIcon(localState)).toBe('angle-double-left'); }); }); diff --git a/spec/javascripts/ide/stores/modules/commit/actions_spec.js b/spec/javascripts/ide/stores/modules/commit/actions_spec.js index 1946a0c547c..116967208e0 100644 --- a/spec/javascripts/ide/stores/modules/commit/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/commit/actions_spec.js @@ -209,14 +209,14 @@ describe('IDE commit module actions', () => { }, }, }; - store.state.changedFiles.push(f, { + store.state.stagedFiles.push(f, { ...file('changedFile2'), changed: true, }); - store.state.openFiles = store.state.changedFiles; + store.state.openFiles = store.state.stagedFiles; - store.state.changedFiles.forEach(changedFile => { - store.state.entries[changedFile.path] = changedFile; + store.state.stagedFiles.forEach(stagedFile => { + store.state.entries[stagedFile.path] = stagedFile; }); }); @@ -248,19 +248,6 @@ describe('IDE commit module actions', () => { .catch(done.fail); }); - it('removes all changed files', done => { - store - .dispatch('commit/updateFilesAfterCommit', { - data, - branch, - }) - .then(() => { - expect(store.state.changedFiles.length).toBe(0); - }) - .then(done) - .catch(done.fail); - }); - it('sets files commit data', done => { store .dispatch('commit/updateFilesAfterCommit', { @@ -294,10 +281,10 @@ describe('IDE commit module actions', () => { branch, }) .then(() => { - expect(eventHub.$emit).toHaveBeenCalledWith( - `editor.update.model.content.${f.path}`, - f.content, - ); + expect(eventHub.$emit).toHaveBeenCalledWith(`editor.update.model.content.${f.key}`, { + content: f.content, + changed: false, + }); }) .then(done) .catch(done.fail); @@ -335,12 +322,22 @@ describe('IDE commit module actions', () => { }, }, }; - store.state.changedFiles.push(file('changed')); - store.state.changedFiles[0].active = true; + + const f = { + ...file('changed'), + type: 'blob', + active: true, + }; + store.state.stagedFiles.push(f); + store.state.changedFiles = [ + { + ...f, + }, + ]; store.state.openFiles = store.state.changedFiles; - store.state.openFiles.forEach(f => { - store.state.entries[f.path] = f; + store.state.openFiles.forEach(localF => { + store.state.entries[localF.path] = localF; }); store.state.commit.commitAction = '2'; @@ -420,11 +417,13 @@ describe('IDE commit module actions', () => { .catch(done.fail); }); - it('adds commit data to changed files', done => { + it('adds commit data to files', done => { store .dispatch('commit/commitChanges') .then(() => { - expect(store.state.openFiles[0].lastCommit.message).toBe('test message'); + expect(store.state.entries[store.state.openFiles[0].path].lastCommit.message).toBe( + 'test message', + ); done(); }) @@ -443,6 +442,16 @@ describe('IDE commit module actions', () => { .catch(done.fail); }); + it('removes all staged files', done => { + store + .dispatch('commit/commitChanges') + .then(() => { + expect(store.state.stagedFiles.length).toBe(0); + }) + .then(done) + .catch(done.fail); + }); + describe('merge request', () => { it('redirects to new merge request page', done => { spyOn(eventHub, '$on'); @@ -471,7 +480,7 @@ describe('IDE commit module actions', () => { store .dispatch('commit/commitChanges') .then(() => { - expect(store.state.changedFiles.length).toBe(0); + expect(store.state.stagedFiles.length).toBe(0); done(); }) diff --git a/spec/javascripts/ide/stores/modules/commit/getters_spec.js b/spec/javascripts/ide/stores/modules/commit/getters_spec.js index e396284ec2c..55580f046ad 100644 --- a/spec/javascripts/ide/stores/modules/commit/getters_spec.js +++ b/spec/javascripts/ide/stores/modules/commit/getters_spec.js @@ -34,17 +34,17 @@ describe('IDE commit module getters', () => { discardDraftButtonDisabled: false, }; const rootState = { - changedFiles: ['a'], + stagedFiles: ['a'], }; - it('returns false when discardDraftButtonDisabled is false & changedFiles is not empty', () => { + it('returns false when discardDraftButtonDisabled is false & stagedFiles is not empty', () => { expect( getters.commitButtonDisabled(state, localGetters, rootState), ).toBeFalsy(); }); - it('returns true when discardDraftButtonDisabled is false & changedFiles is empty', () => { - rootState.changedFiles.length = 0; + it('returns true when discardDraftButtonDisabled is false & stagedFiles is empty', () => { + rootState.stagedFiles.length = 0; expect( getters.commitButtonDisabled(state, localGetters, rootState), @@ -61,7 +61,7 @@ describe('IDE commit module getters', () => { it('returns true when discardDraftButtonDisabled is false & changedFiles is not empty', () => { localGetters.discardDraftButtonDisabled = false; - rootState.changedFiles.length = 0; + rootState.stagedFiles.length = 0; expect( getters.commitButtonDisabled(state, localGetters, rootState), diff --git a/spec/javascripts/ide/stores/mutations/file_spec.js b/spec/javascripts/ide/stores/mutations/file_spec.js index bf9d5166d0a..6fba934810d 100644 --- a/spec/javascripts/ide/stores/mutations/file_spec.js +++ b/spec/javascripts/ide/stores/mutations/file_spec.js @@ -8,7 +8,10 @@ describe('IDE store file mutations', () => { beforeEach(() => { localState = state(); - localFile = file(); + localFile = { + ...file(), + type: 'blob', + }; localState.entries[localFile.path] = localFile; }); @@ -183,6 +186,49 @@ describe('IDE store file mutations', () => { }); }); + describe('STAGE_CHANGE', () => { + it('adds file into stagedFiles array', () => { + mutations.STAGE_CHANGE(localState, localFile.path); + + expect(localState.stagedFiles.length).toBe(1); + expect(localState.stagedFiles[0]).toEqual(localFile); + }); + + it('updates stagedFile if it is already staged', () => { + mutations.STAGE_CHANGE(localState, localFile.path); + + localFile.raw = 'testing 123'; + + mutations.STAGE_CHANGE(localState, localFile.path); + + expect(localState.stagedFiles.length).toBe(1); + expect(localState.stagedFiles[0].raw).toEqual('testing 123'); + }); + }); + + describe('UNSTAGE_CHANGE', () => { + let f; + + beforeEach(() => { + f = { + ...file(), + type: 'blob', + staged: true, + }; + + localState.stagedFiles.push(f); + localState.changedFiles.push(f); + localState.entries[f.path] = f; + }); + + it('removes from stagedFiles array', () => { + mutations.UNSTAGE_CHANGE(localState, f.path); + + expect(localState.stagedFiles.length).toBe(0); + expect(localState.changedFiles.length).toBe(1); + }); + }); + describe('TOGGLE_FILE_CHANGED', () => { it('updates file changed status', () => { mutations.TOGGLE_FILE_CHANGED(localState, { diff --git a/spec/javascripts/ide/stores/mutations_spec.js b/spec/javascripts/ide/stores/mutations_spec.js index 9edfc080bec..575039e755e 100644 --- a/spec/javascripts/ide/stores/mutations_spec.js +++ b/spec/javascripts/ide/stores/mutations_spec.js @@ -69,6 +69,16 @@ describe('Multi-file store mutations', () => { }); }); + describe('CLEAR_STAGED_CHANGES', () => { + it('clears stagedFiles array', () => { + localState.stagedFiles.push('a'); + + mutations.CLEAR_STAGED_CHANGES(localState); + + expect(localState.stagedFiles.length).toBe(0); + }); + }); + describe('UPDATE_VIEWER', () => { it('sets viewer state', () => { mutations.UPDATE_VIEWER(localState, 'diff'); diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 451b038e9a6..fcdc31c8984 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1384,29 +1384,51 @@ describe Ci::Build do end end - describe '#update_project_statistics' do - let!(:build) { create(:ci_build, artifacts_size: 23) } - - it 'updates project statistics when the artifact size changes' do - expect(ProjectCacheWorker).to receive(:perform_async) - .with(build.project_id, [], [:build_artifacts_size]) + context 'when updating the build' do + let(:build) { create(:ci_build, artifacts_size: 23) } + it 'updates project statistics' do build.artifacts_size = 42 - build.save! + + expect(build).to receive(:update_project_statistics_after_save).and_call_original + + expect { build.save! } + .to change { build.project.statistics.reload.build_artifacts_size } + .by(19) end - it 'does not update project statistics when the artifact size stays the same' do - expect(ProjectCacheWorker).not_to receive(:perform_async) + context 'when the artifact size stays the same' do + it 'does not update project statistics' do + build.name = 'changed' - build.name = 'changed' - build.save! + expect(build).not_to receive(:update_project_statistics_after_save) + + build.save! + end end + end - it 'updates project statistics when the build is destroyed' do - expect(ProjectCacheWorker).to receive(:perform_async) - .with(build.project_id, [], [:build_artifacts_size]) + context 'when destroying the build' do + let!(:build) { create(:ci_build, artifacts_size: 23) } + + it 'updates project statistics' do + expect(ProjectStatistics) + .to receive(:increment_statistic) + .and_call_original + + expect { build.destroy! } + .to change { build.project.statistics.reload.build_artifacts_size } + .by(-23) + end + + context 'when the build is destroyed due to the project being destroyed' do + it 'does not update the project statistics' do + expect(ProjectStatistics) + .not_to receive(:increment_statistic) - build.destroy + build.project.update_attributes(pending_delete: true) + build.project.destroy! + end end end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 1aa28434879..a3e119cbc27 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Ci::JobArtifact do - set(:artifact) { create(:ci_job_artifact, :archive) } + let(:artifact) { create(:ci_job_artifact, :archive) } describe "Associations" do it { is_expected.to belong_to(:project) } @@ -59,10 +59,32 @@ describe Ci::JobArtifact do end end - describe '#set_size' do - it 'sets the size' do + context 'creating the artifact' do + let(:project) { create(:project) } + let(:artifact) { create(:ci_job_artifact, :archive, project: project) } + + it 'sets the size from the file size' do expect(artifact.size).to eq(106365) end + + it 'updates the project statistics' do + expect { artifact } + .to change { project.statistics.reload.build_artifacts_size } + .by(106365) + end + end + + context 'updating the artifact file' do + it 'updates the artifact size' do + artifact.update!(file: fixture_file_upload(File.join(Rails.root, 'spec/fixtures/dk.png'))) + expect(artifact.size).to eq(1062) + end + + it 'updates the project statistics' do + expect { artifact.update!(file: fixture_file_upload(File.join(Rails.root, 'spec/fixtures/dk.png'))) } + .to change { artifact.project.statistics.reload.build_artifacts_size } + .by(1062 - 106365) + end end describe '#file' do @@ -118,4 +140,71 @@ describe Ci::JobArtifact do is_expected.to be_nil end end + + context 'when destroying the artifact' do + let(:project) { create(:project, :repository) } + let(:pipeline) { create(:ci_pipeline, project: project) } + let!(:build) { create(:ci_build, :artifacts, pipeline: pipeline) } + + it 'updates the project statistics' do + artifact = build.job_artifacts.first + + expect(ProjectStatistics) + .to receive(:increment_statistic) + .and_call_original + + expect { artifact.destroy } + .to change { project.statistics.reload.build_artifacts_size } + .by(-106365) + end + + context 'when it is destroyed from the project level' do + it 'does not update the project statistics' do + expect(ProjectStatistics) + .not_to receive(:increment_statistic) + + project.update_attributes(pending_delete: true) + project.destroy! + end + end + end + + describe 'file is being stored' do + subject { create(:ci_job_artifact, :archive) } + + context 'when object has nil store' do + before do + subject.update_column(:file_store, nil) + subject.reload + end + + it 'is stored locally' do + expect(subject.file_store).to be(nil) + expect(subject.file).to be_file_storage + expect(subject.file.object_store).to eq(ObjectStorage::Store::LOCAL) + end + end + + context 'when existing object has local store' do + it 'is stored locally' do + expect(subject.file_store).to be(ObjectStorage::Store::LOCAL) + expect(subject.file).to be_file_storage + expect(subject.file.object_store).to eq(ObjectStorage::Store::LOCAL) + end + end + + context 'when direct upload is enabled' do + before do + stub_artifacts_object_storage(direct_upload: true) + end + + context 'when file is stored' do + it 'is stored remotely' do + expect(subject.file_store).to eq(ObjectStorage::Store::REMOTE) + expect(subject.file).not_to be_file_storage + expect(subject.file.object_store).to eq(ObjectStorage::Store::REMOTE) + end + end + end + end end diff --git a/spec/models/lfs_object_spec.rb b/spec/models/lfs_object_spec.rb index a182116d637..ba06ff42d87 100644 --- a/spec/models/lfs_object_spec.rb +++ b/spec/models/lfs_object_spec.rb @@ -81,5 +81,44 @@ describe LfsObject do end end end + + describe 'file is being stored' do + let(:lfs_object) { create(:lfs_object, :with_file) } + + context 'when object has nil store' do + before do + lfs_object.update_column(:file_store, nil) + lfs_object.reload + end + + it 'is stored locally' do + expect(lfs_object.file_store).to be(nil) + expect(lfs_object.file).to be_file_storage + expect(lfs_object.file.object_store).to eq(ObjectStorage::Store::LOCAL) + end + end + + context 'when existing object has local store' do + it 'is stored locally' do + expect(lfs_object.file_store).to be(ObjectStorage::Store::LOCAL) + expect(lfs_object.file).to be_file_storage + expect(lfs_object.file.object_store).to eq(ObjectStorage::Store::LOCAL) + end + end + + context 'when direct upload is enabled' do + before do + stub_lfs_object_storage(direct_upload: true) + end + + context 'when file is stored' do + it 'is stored remotely' do + expect(lfs_object.file_store).to eq(ObjectStorage::Store::REMOTE) + expect(lfs_object.file).not_to be_file_storage + expect(lfs_object.file.object_store).to eq(ObjectStorage::Store::REMOTE) + end + end + end + end end end diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index 5cff2af4aca..38a3590ad12 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -4,26 +4,6 @@ describe ProjectStatistics do let(:project) { create :project } let(:statistics) { project.statistics } - describe 'constants' do - describe 'STORAGE_COLUMNS' do - it 'is an array of symbols' do - expect(described_class::STORAGE_COLUMNS).to be_kind_of Array - expect(described_class::STORAGE_COLUMNS.map(&:class).uniq).to eq [Symbol] - end - end - - describe 'STATISTICS_COLUMNS' do - it 'is an array of symbols' do - expect(described_class::STATISTICS_COLUMNS).to be_kind_of Array - expect(described_class::STATISTICS_COLUMNS.map(&:class).uniq).to eq [Symbol] - end - - it 'includes all storage columns' do - expect(described_class::STATISTICS_COLUMNS & described_class::STORAGE_COLUMNS).to eq described_class::STORAGE_COLUMNS - end - end - end - describe 'associations' do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:namespace) } @@ -63,7 +43,6 @@ describe ProjectStatistics do allow(statistics).to receive(:update_commit_count) allow(statistics).to receive(:update_repository_size) allow(statistics).to receive(:update_lfs_objects_size) - allow(statistics).to receive(:update_build_artifacts_size) allow(statistics).to receive(:update_storage_size) end @@ -76,7 +55,6 @@ describe ProjectStatistics do expect(statistics).to have_received(:update_commit_count) expect(statistics).to have_received(:update_repository_size) expect(statistics).to have_received(:update_lfs_objects_size) - expect(statistics).to have_received(:update_build_artifacts_size) end end @@ -89,7 +67,6 @@ describe ProjectStatistics do expect(statistics).to have_received(:update_lfs_objects_size) expect(statistics).not_to have_received(:update_commit_count) expect(statistics).not_to have_received(:update_repository_size) - expect(statistics).not_to have_received(:update_build_artifacts_size) end end end @@ -131,40 +108,6 @@ describe ProjectStatistics do end end - describe '#update_build_artifacts_size' do - let!(:pipeline) { create(:ci_pipeline, project: project) } - - context 'when new job artifacts are calculated' do - let(:ci_build) { create(:ci_build, pipeline: pipeline) } - - before do - create(:ci_job_artifact, :archive, project: pipeline.project, job: ci_build) - end - - it "stores the size of related build artifacts" do - statistics.update_build_artifacts_size - - expect(statistics.build_artifacts_size).to be(106365) - end - - it 'calculates related build artifacts by project' do - expect(Ci::JobArtifact).to receive(:artifacts_size_for).with(project) { 0 } - - statistics.update_build_artifacts_size - end - end - - context 'when legacy artifacts are used' do - let!(:ci_build) { create(:ci_build, pipeline: pipeline, artifacts_size: 10.megabytes) } - - it "stores the size of related build artifacts" do - statistics.update_build_artifacts_size - - expect(statistics.build_artifacts_size).to eq(10.megabytes) - end - end - end - describe '#update_storage_size' do it "sums all storage counters" do statistics.update!( @@ -177,4 +120,27 @@ describe ProjectStatistics do expect(statistics.storage_size).to eq 5 end end + + describe '.increment_statistic' do + it 'increases the statistic by that amount' do + expect { described_class.increment_statistic(project.id, :build_artifacts_size, 13) } + .to change { statistics.reload.build_artifacts_size } + .by(13) + end + + context 'when the amount is 0' do + it 'does not execute a query' do + project + expect { described_class.increment_statistic(project.id, :build_artifacts_size, 0) } + .not_to exceed_query_limit(0) + end + end + + context 'when using an invalid column' do + it 'raises an error' do + expect { described_class.increment_statistic(project.id, :id, 13) } + .to raise_error(ArgumentError, "Cannot increment attribute: id") + end + end + end end diff --git a/spec/services/labels/transfer_service_spec.rb b/spec/services/labels/transfer_service_spec.rb index ae819c011de..80bac590a11 100644 --- a/spec/services/labels/transfer_service_spec.rb +++ b/spec/services/labels/transfer_service_spec.rb @@ -8,6 +8,7 @@ describe Labels::TransferService do let(:group_3) { create(:group) } let(:project_1) { create(:project, namespace: group_2) } let(:project_2) { create(:project, namespace: group_3) } + let(:project_3) { create(:project, namespace: group_1) } let(:group_label_1) { create(:group_label, group: group_1, name: 'Group Label 1') } let(:group_label_2) { create(:group_label, group: group_1, name: 'Group Label 2') } @@ -23,6 +24,7 @@ describe Labels::TransferService do create(:labeled_issue, project: project_1, labels: [group_label_4]) create(:labeled_issue, project: project_1, labels: [project_label_1]) create(:labeled_issue, project: project_2, labels: [group_label_5]) + create(:labeled_issue, project: project_3, labels: [group_label_1]) create(:labeled_merge_request, source_project: project_1, labels: [group_label_1, group_label_2]) create(:labeled_merge_request, source_project: project_2, labels: [group_label_5]) end @@ -52,5 +54,13 @@ describe Labels::TransferService do expect(project_1.labels.where(title: group_label_4.title)).to be_empty end + + it 'updates only label links in the given project' do + service.execute + + targets = LabelLink.where(label_id: group_label_1.id).map(&:target) + + expect(targets).to eq(project_3.issues) + end end end diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index 16455e2517b..e7277b337f6 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -75,36 +75,8 @@ describe ObjectStorage do expect(object).to receive(:file_store).and_return(nil) end - context 'when object storage is enabled' do - context 'when direct uploads are enabled' do - before do - stub_uploads_object_storage(uploader_class, enabled: true, direct_upload: true) - end - - it "uses Store::REMOTE" do - is_expected.to eq(described_class::Store::REMOTE) - end - end - - context 'when direct uploads are disabled' do - before do - stub_uploads_object_storage(uploader_class, enabled: true, direct_upload: false) - end - - it "uses Store::LOCAL" do - is_expected.to eq(described_class::Store::LOCAL) - end - end - end - - context 'when object storage is disabled' do - before do - stub_uploads_object_storage(uploader_class, enabled: false) - end - - it "uses Store::LOCAL" do - is_expected.to eq(described_class::Store::LOCAL) - end + it "uses Store::LOCAL" do + is_expected.to eq(described_class::Store::LOCAL) end end @@ -537,6 +509,72 @@ describe ObjectStorage do end end + context 'when local file is used' do + let(:temp_file) { Tempfile.new("test") } + + before do + FileUtils.touch(temp_file) + end + + after do + FileUtils.rm_f(temp_file) + end + + context 'when valid file is used' do + context 'when valid file is specified' do + let(:uploaded_file) { temp_file } + + context 'when object storage and direct upload is specified' do + before do + stub_uploads_object_storage(uploader_class, enabled: true, direct_upload: true) + end + + context 'when file is stored' do + subject do + uploader.store!(uploaded_file) + end + + it 'file to be remotely stored in permament location' do + subject + + expect(uploader).to be_exists + expect(uploader).not_to be_cached + expect(uploader).not_to be_file_storage + expect(uploader.path).not_to be_nil + expect(uploader.path).not_to include('tmp/upload') + expect(uploader.path).not_to include('tmp/cache') + expect(uploader.object_store).to eq(described_class::Store::REMOTE) + end + end + end + + context 'when object storage and direct upload is not used' do + before do + stub_uploads_object_storage(uploader_class, enabled: true, direct_upload: false) + end + + context 'when file is stored' do + subject do + uploader.store!(uploaded_file) + end + + it 'file to be remotely stored in permament location' do + subject + + expect(uploader).to be_exists + expect(uploader).not_to be_cached + expect(uploader).to be_file_storage + expect(uploader.path).not_to be_nil + expect(uploader.path).not_to include('tmp/upload') + expect(uploader.path).not_to include('tmp/cache') + expect(uploader.object_store).to eq(described_class::Store::LOCAL) + end + end + end + end + end + end + context 'when remote file is used' do let(:temp_file) { Tempfile.new("test") } @@ -590,9 +628,9 @@ describe ObjectStorage do expect(uploader).to be_exists expect(uploader).to be_cached + expect(uploader).not_to be_file_storage expect(uploader.path).not_to be_nil expect(uploader.path).not_to include('tmp/cache') - expect(uploader.url).not_to be_nil expect(uploader.path).not_to include('tmp/cache') expect(uploader.object_store).to eq(described_class::Store::REMOTE) end @@ -607,6 +645,7 @@ describe ObjectStorage do expect(uploader).to be_exists expect(uploader).not_to be_cached + expect(uploader).not_to be_file_storage expect(uploader.path).not_to be_nil expect(uploader.path).not_to include('tmp/upload') expect(uploader.path).not_to include('tmp/cache') |