diff options
author | Phil Hughes <me@iamphill.com> | 2018-05-30 09:40:37 +0100 |
---|---|---|
committer | Phil Hughes <me@iamphill.com> | 2018-05-30 09:40:37 +0100 |
commit | 6c9844c10e23e939a08a9ba3e8972dd3f112f206 (patch) | |
tree | 13f98e7310752a5a73cce5f72fc4f1f042b6b2fb | |
parent | e6e9706e5308b366ce20ff592d0685fa02e4b404 (diff) | |
parent | fd79df64c5411308e67a62b4c02a07f5317ddec1 (diff) | |
download | gitlab-ce-6c9844c10e23e939a08a9ba3e8972dd3f112f206.tar.gz |
Merge branch 'master' into ide-jobs-list-components
158 files changed, 1721 insertions, 486 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ef263a3f106..0b2ee4b1cd8 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -6,7 +6,7 @@ image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.3.7-golang-1.9-git - gitlab-org .default-cache: &default-cache - key: "ruby-2.3.7-with-yarn" + key: "ruby-2.3.7-debian-stretch-with-yarn" paths: - vendor/ruby - .yarn-cache/ @@ -550,7 +550,7 @@ static-analysis: script: - scripts/static-analysis cache: - key: "ruby-2.3.7-with-yarn-and-rubocop" + key: "ruby-2.3.7-debian-stretch-with-yarn-and-rubocop" paths: - vendor/ruby - .yarn-cache/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 99cf96035d9..9bc941cafaa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 10.8.2 (2018-05-28) + +### Security (3 changes) + +- Prevent user passwords from being changed without providing the previous password. +- Fix API to remove deploy key from project instead of deleting it entirely. +- Fixed bug that allowed importing arbitrary project attributes. + + ## 10.8.1 (2018-05-23) ### Fixed (9 changes) @@ -193,6 +202,15 @@ entry. - Gitaly handles repository forks by default. +## 10.7.5 (2018-05-28) + +### Security (3 changes) + +- Prevent user passwords from being changed without providing the previous password. +- Fix API to remove deploy key from project instead of deleting it entirely. +- Fixed bug that allowed importing arbitrary project attributes. + + ## 10.7.4 (2018-05-21) ### Fixed (1 change) @@ -457,6 +475,16 @@ entry. - Upgrade Gitaly to upgrade its charlock_holmes. +## 10.6.6 (2018-05-28) + +### Security (4 changes) + +- Do not allow non-members to create MRs via forked projects when MRs are private. +- Prevent user passwords from being changed without providing the previous password. +- Fix API to remove deploy key from project instead of deleting it entirely. +- Fixed bug that allowed importing arbitrary project attributes. + + ## 10.6.5 (2018-04-24) ### Security (1 change) @@ -133,7 +133,7 @@ gem 'gitlab-markup', '~> 1.6.2' gem 'redcarpet', '~> 3.4' gem 'commonmarker', '~> 0.17' gem 'RedCloth', '~> 4.3.2' -gem 'rdoc', '~> 4.2' +gem 'rdoc', '~> 6.0' gem 'org-ruby', '~> 0.9.12' gem 'creole', '~> 0.5.0' gem 'wikicloth', '0.8.1' @@ -320,7 +320,7 @@ group :development, :test do gem 'pry-byebug', '~> 3.4.1', platform: :mri gem 'pry-rails', '~> 0.3.4' - gem 'awesome_print', '~> 1.2.0', require: false + gem 'awesome_print', '~> 1.8.0', require: false gem 'fuubar', '~> 2.2.0' gem 'database_cleaner', '~> 1.5.0' diff --git a/Gemfile.lock b/Gemfile.lock index b0b7bb537a8..cfd74334c86 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -69,7 +69,7 @@ GEM attr_encrypted (3.1.0) encryptor (~> 3.0.0) attr_required (1.0.0) - awesome_print (1.2.0) + awesome_print (1.8.0) axiom-types (0.1.1) descendants_tracker (~> 0.0.4) ice_nine (~> 0.11.0) @@ -694,8 +694,7 @@ GEM ffi rbnacl-libsodium (1.0.11) rbnacl (>= 3.0.1) - rdoc (4.2.2) - json (~> 1.4) + rdoc (6.0.4) re2 (1.1.1) recaptcha (3.0.0) json @@ -801,7 +800,7 @@ GEM rubyzip (1.2.1) rufus-scheduler (3.4.0) et-orbi (~> 1.0) - rugged (0.27.0) + rugged (0.27.1) safe_yaml (1.0.4) sanitize (2.1.0) nokogiri (>= 1.4.4) @@ -978,7 +977,7 @@ DEPENDENCIES asciidoctor-plantuml (= 0.0.8) asset_sync (~> 2.4) attr_encrypted (~> 3.1.0) - awesome_print (~> 1.2.0) + awesome_print (~> 1.8.0) babosa (~> 1.0.2) base32 (~> 0.3.0) batch-loader (~> 1.2.1) @@ -1124,7 +1123,7 @@ DEPENDENCIES rblineprof (~> 0.3.6) rbnacl (~> 4.0) rbnacl-libsodium - rdoc (~> 4.2) + rdoc (~> 6.0) re2 (~> 1.1.1) recaptcha (~> 3.0) redcarpet (~> 3.4) diff --git a/app/assets/javascripts/badges/components/badge_form.vue b/app/assets/javascripts/badges/components/badge_form.vue index ae942b2c1a7..5975cb9669e 100644 --- a/app/assets/javascripts/badges/components/badge_form.vue +++ b/app/assets/javascripts/badges/components/badge_form.vue @@ -160,7 +160,7 @@ export default { @input="debouncedPreview" /> <span - class="help-block" + class="form-text text-muted" v-html="helpText" ></span> </div> @@ -176,7 +176,7 @@ export default { @input="debouncedPreview" /> <span - class="help-block" + class="form-text text-muted" v-html="helpText" ></span> </div> diff --git a/app/assets/javascripts/ide/components/changed_file_icon.vue b/app/assets/javascripts/ide/components/changed_file_icon.vue index 1cec84706fc..a4e06bbbe3c 100644 --- a/app/assets/javascripts/ide/components/changed_file_icon.vue +++ b/app/assets/javascripts/ide/components/changed_file_icon.vue @@ -43,7 +43,7 @@ export default { return `${this.changedIcon}-solid`; }, changedIconClass() { - return `multi-${this.changedIcon} pull-left`; + return `multi-${this.changedIcon} float-left`; }, tooltipTitle() { if (!this.showTooltip) return undefined; diff --git a/app/assets/javascripts/ide/components/commit_sidebar/form.vue b/app/assets/javascripts/ide/components/commit_sidebar/form.vue index 81961fe3c57..705953c86e3 100644 --- a/app/assets/javascripts/ide/components/commit_sidebar/form.vue +++ b/app/assets/javascripts/ide/components/commit_sidebar/form.vue @@ -144,14 +144,14 @@ export default { <loading-button :loading="submitCommitLoading" :disabled="commitButtonDisabled" - container-class="btn btn-success btn-sm pull-left" + container-class="btn btn-success btn-sm float-left" :label="__('Commit')" @click="commitChanges" /> <button v-if="!discardDraftButtonDisabled" type="button" - class="btn btn-default btn-sm pull-right" + class="btn btn-default btn-sm float-right" @click="discardDraft" > {{ __('Discard draft') }} @@ -159,7 +159,7 @@ export default { <button v-else type="button" - class="btn btn-default btn-sm pull-right" + class="btn btn-default btn-sm float-right" @click="toggleIsSmall" > {{ __('Collapse') }} diff --git a/app/assets/javascripts/ide/components/commit_sidebar/list.vue b/app/assets/javascripts/ide/components/commit_sidebar/list.vue index c3ac18bfb83..1325fc993b2 100644 --- a/app/assets/javascripts/ide/components/commit_sidebar/list.vue +++ b/app/assets/javascripts/ide/components/commit_sidebar/list.vue @@ -120,7 +120,7 @@ export default { </ul> <p v-else - class="multi-file-commit-list help-block" + class="multi-file-commit-list form-text text-muted" > {{ __('No changes') }} </p> diff --git a/app/assets/javascripts/ide/components/commit_sidebar/message_field.vue b/app/assets/javascripts/ide/components/commit_sidebar/message_field.vue index dcd934f76b7..f14fcdc88ed 100644 --- a/app/assets/javascripts/ide/components/commit_sidebar/message_field.vue +++ b/app/assets/javascripts/ide/components/commit_sidebar/message_field.vue @@ -80,7 +80,7 @@ export default { {{ __('Commit Message') }} <span v-popover="$options.popoverOptions" - class="help-block prepend-left-10" + class="form-text text-muted prepend-left-10" > <icon name="question" diff --git a/app/assets/javascripts/ide/components/new_dropdown/modal.vue b/app/assets/javascripts/ide/components/new_dropdown/modal.vue index d83a90f71e1..dd2800179ff 100644 --- a/app/assets/javascripts/ide/components/new_dropdown/modal.vue +++ b/app/assets/javascripts/ide/components/new_dropdown/modal.vue @@ -72,21 +72,19 @@ export default { <form slot="body" @submit.prevent="createEntryInStore" - class="form-group row append-bottom-0" + class="form-group row" > - <fieldset class="form-group append-bottom-0"> - <label class="label-light col-form-label col-sm-3 ide-new-modal-label"> - {{ __('Name') }} - </label> - <div class="col-sm-9"> - <input - type="text" - class="form-control" - v-model="entryName" - ref="fieldName" - /> - </div> - </fieldset> + <label class="label-light col-form-label col-sm-3"> + {{ __('Name') }} + </label> + <div class="col-sm-9"> + <input + type="text" + class="form-control" + v-model="entryName" + ref="fieldName" + /> + </div> </form> </deprecated-modal> </template> diff --git a/app/assets/javascripts/ide/components/repo_file.vue b/app/assets/javascripts/ide/components/repo_file.vue index 442697e1c80..f56aeced806 100644 --- a/app/assets/javascripts/ide/components/repo_file.vue +++ b/app/assets/javascripts/ide/components/repo_file.vue @@ -169,7 +169,7 @@ export default { :show-tooltip="true" :show-staged-icon="true" :force-modified-icon="true" - class="pull-right" + class="float-right" /> </span> <new-dropdown diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 3548c07aea8..bac7d966ecc 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -362,7 +362,7 @@ export default class MergeRequestTabs { // // status - Boolean, true to show, false to hide toggleLoading(status) { - $('.mr-loading-status .loading').toggleClass('hidden', status); + $('.mr-loading-status .loading').toggleClass('hidden', !status); } diffViewType() { diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue index 755a34b7348..06b0ab184ed 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue +++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue @@ -213,7 +213,7 @@ </i> </div> </div> - <span class="help-block">{{ visibilityLevelDescription }}</span> + <span class="form-text text-muted">{{ visibilityLevelDescription }}</span> <label v-if="visibilityLevel !== visibilityOptions.PRIVATE" class="request-access" diff --git a/app/assets/javascripts/pages/users/user_tabs.js b/app/assets/javascripts/pages/users/user_tabs.js index ca375007ec5..9404b06615e 100644 --- a/app/assets/javascripts/pages/users/user_tabs.js +++ b/app/assets/javascripts/pages/users/user_tabs.js @@ -77,10 +77,9 @@ export default class UserTabs { this.action = action || this.defaultAction; this.$parentEl = $(parentEl) || $(document); this.windowLocation = window.location; - this.$parentEl.find('.nav-links a') - .each((i, navLink) => { - this.loaded[$(navLink).attr('data-action')] = false; - }); + this.$parentEl.find('.nav-links a').each((i, navLink) => { + this.loaded[$(navLink).attr('data-action')] = false; + }); this.actions = Object.keys(this.loaded); this.bindEvents(); @@ -116,8 +115,7 @@ export default class UserTabs { } activateTab(action) { - return this.$parentEl.find(`.nav-links .js-${action}-tab a`) - .tab('show'); + return this.$parentEl.find(`.nav-links .js-${action}-tab a`).tab('show'); } setTab(action, endpoint) { @@ -137,7 +135,8 @@ export default class UserTabs { loadTab(action, endpoint) { this.toggleLoading(true); - return axios.get(endpoint) + return axios + .get(endpoint) .then(({ data }) => { const tabSelector = `div#${action}`; this.$parentEl.find(tabSelector).html(data.html); @@ -161,10 +160,11 @@ export default class UserTabs { const utcOffset = $calendarWrap.data('utcOffset'); let utcFormatted = 'UTC'; if (utcOffset !== 0) { - utcFormatted = `UTC${utcOffset > 0 ? '+' : ''}${(utcOffset / 3600)}`; + utcFormatted = `UTC${utcOffset > 0 ? '+' : ''}${utcOffset / 3600}`; } - axios.get(calendarPath) + axios + .get(calendarPath) .then(({ data }) => { $calendarWrap.html(CALENDAR_TEMPLATE); $calendarWrap.find('.calendar-hint').append(`(Timezone: ${utcFormatted})`); @@ -180,17 +180,20 @@ export default class UserTabs { } toggleLoading(status) { - return this.$parentEl.find('.loading-status .loading') - .toggleClass('hidden', status); + return this.$parentEl.find('.loading-status .loading').toggleClass('hidden', !status); } setCurrentAction(source) { let newState = source; newState = newState.replace(/\/+$/, ''); newState += this.windowLocation.search + this.windowLocation.hash; - history.replaceState({ - url: newState, - }, document.title, newState); + history.replaceState( + { + url: newState, + }, + document.title, + newState, + ); return newState; } diff --git a/app/assets/javascripts/profile/account/components/update_username.vue b/app/assets/javascripts/profile/account/components/update_username.vue index a7a2a7235fd..b37febe523c 100644 --- a/app/assets/javascripts/profile/account/components/update_username.vue +++ b/app/assets/javascripts/profile/account/components/update_username.vue @@ -99,7 +99,7 @@ Please update your Git repository remotes as soon as possible.`), :disabled="isRequestPending" /> </div> - <p class="help-block"> + <p class="form-text text-muted"> {{ path }} </p> </div> diff --git a/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_machine_type_dropdown.vue b/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_machine_type_dropdown.vue index 6be39702546..ab7d2d41ece 100644 --- a/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_machine_type_dropdown.vue +++ b/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_machine_type_dropdown.vue @@ -132,7 +132,7 @@ export default { </div> </div> <span - class="help-block" + class="form-text text-muted" :class="{ 'gl-field-error': hasErrors }" v-if="hasErrors" > diff --git a/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_project_id_dropdown.vue b/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_project_id_dropdown.vue index f813a4625d6..25350ef0fa9 100644 --- a/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_project_id_dropdown.vue +++ b/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_project_id_dropdown.vue @@ -193,7 +193,7 @@ export default { </div> </div> <span - class="help-block" + class="form-text text-muted" :class="{ 'gl-field-error': hasErrors }" v-html="helpText" ></span> diff --git a/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_zone_dropdown.vue b/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_zone_dropdown.vue index 0c63ff5dc63..8ee4eefcd91 100644 --- a/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_zone_dropdown.vue +++ b/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_zone_dropdown.vue @@ -106,7 +106,7 @@ export default { </div> </div> <span - class="help-block" + class="form-text text-muted" :class="{ 'gl-field-error': hasErrors }" v-if="hasErrors" > diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_squash_before_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_squash_before_merge.vue index 926a3172412..875c3323dbb 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_squash_before_merge.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_squash_before_merge.vue @@ -1,15 +1,63 @@ -/* -The squash-before-merge button is EE only, but it's located right in the middle -of the readyToMerge state component template. - -If we didn't declare this component in CE, we'd need to maintain a separate copy -of the readyToMergeState template in EE, which is pretty big and likely to change. - -Instead, in CE, we declare the component, but it's hidden and is configured to do nothing. -In EE, the configuration extends this object to add a functioning squash-before-merge -button. -*/ - <script> - export default {}; +import Icon from '~/vue_shared/components/icon.vue'; +import eventHub from '~/vue_merge_request_widget/event_hub'; +import tooltip from '~/vue_shared/directives/tooltip'; + +export default { + components: { + Icon, + }, + directives: { + tooltip, + }, + props: { + mr: { + type: Object, + required: true, + }, + isMergeButtonDisabled: { + type: Boolean, + required: true, + }, + }, + data() { + return { + squashBeforeMerge: this.mr.squash, + }; + }, + methods: { + updateSquashModel() { + eventHub.$emit('MRWidgetUpdateSquash', this.squashBeforeMerge); + }, + }, +}; </script> + +<template> + <div class="accept-control inline"> + <label class="merge-param-checkbox"> + <input + type="checkbox" + name="squash" + class="qa-squash-checkbox" + :disabled="isMergeButtonDisabled" + v-model="squashBeforeMerge" + @change="updateSquashModel" + /> + {{ __('Squash commits') }} + </label> + <a + :href="mr.squashBeforeMergeHelpPath" + data-title="About this feature" + data-placement="bottom" + target="_blank" + rel="noopener noreferrer nofollow" + data-container="body" + v-tooltip + > + <icon + name="question-o" + /> + </a> + </div> +</template> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue index 1d1c8ebc179..3a194320bd8 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue @@ -6,11 +6,13 @@ import MergeRequest from '../../../merge_request'; import Flash from '../../../flash'; import statusIcon from '../mr_widget_status_icon.vue'; import eventHub from '../../event_hub'; +import SquashBeforeMerge from './mr_widget_squash_before_merge.vue'; export default { name: 'ReadyToMerge', components: { statusIcon, + 'squash-before-merge': SquashBeforeMerge, }, props: { mr: { type: Object, required: true }, @@ -101,6 +103,12 @@ export default { return enableSquashBeforeMerge && commitsCount > 1; }, }, + created() { + eventHub.$on('MRWidgetUpdateSquash', this.handleUpdateSquash); + }, + beforeDestroy() { + eventHub.$off('MRWidgetUpdateSquash', this.handleUpdateSquash); + }, methods: { shouldShowMergeControls() { return this.mr.isMergeAllowed || this.shouldShowMergeWhenPipelineSucceedsText; @@ -128,13 +136,9 @@ export default { commit_message: this.commitMessage, merge_when_pipeline_succeeds: this.setToMergeWhenPipelineSucceeds, should_remove_source_branch: this.removeSourceBranch === true, + squash: this.mr.squash, }; - // Only truthy in EE extension of this component - if (this.setAdditionalParams) { - this.setAdditionalParams(options); - } - this.isMakingRequest = true; this.service.merge(options) .then(res => res.data) @@ -154,6 +158,9 @@ export default { new Flash('Something went wrong. Please try again.'); // eslint-disable-line }); }, + handleUpdateSquash(val) { + this.mr.squash = val; + }, initiateMergePolling() { simplePoll((continuePolling, stopPolling) => { this.handleMergePolling(continuePolling, stopPolling); diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index 83b7b054e6f..e5b7e1f1c68 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -15,6 +15,11 @@ export default class MergeRequestStore { const currentUser = data.current_user; const pipelineStatus = data.pipeline ? data.pipeline.details.status : null; + this.squash = data.squash; + this.squashBeforeMergeHelpPath = this.squashBeforeMergeHelpPath || + data.squash_before_merge_help_path; + this.enableSquashBeforeMerge = this.enableSquashBeforeMerge || true; + this.title = data.title; this.targetBranch = data.target_branch; this.sourceBranch = data.source_branch; diff --git a/app/assets/stylesheets/bootstrap_migration.scss b/app/assets/stylesheets/bootstrap_migration.scss index 3b7ee5c73e6..e1a47f3d686 100644 --- a/app/assets/stylesheets/bootstrap_migration.scss +++ b/app/assets/stylesheets/bootstrap_migration.scss @@ -131,6 +131,10 @@ table { } .card { + .card-title { + margin-bottom: 0; + } + &.card-without-border { @extend .border-0; } @@ -147,3 +151,7 @@ table { .nav-tabs .nav-link { border: 0; } + +pre code { + white-space: pre-wrap; +} diff --git a/app/assets/stylesheets/framework/lists.scss b/app/assets/stylesheets/framework/lists.scss index 17f4958d535..d54490c87c6 100644 --- a/app/assets/stylesheets/framework/lists.scss +++ b/app/assets/stylesheets/framework/lists.scss @@ -2,7 +2,7 @@ * Well styled list * */ -.card-body-list { +.hover-list { position: relative; margin: 0; padding: 0; diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index 06078f1d12e..5d0d59e12f2 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -405,7 +405,7 @@ table.u2f-registrations { margin-right: $gl-padding / 4; } - .label-verification-status { + .badge-verification-status { border-width: 1px; border-style: solid; diff --git a/app/assets/stylesheets/pages/repo.scss b/app/assets/stylesheets/pages/repo.scss index 2845063e3c5..6bbcb15329c 100644 --- a/app/assets/stylesheets/pages/repo.scss +++ b/app/assets/stylesheets/pages/repo.scss @@ -546,7 +546,7 @@ margin-right: 0; } - &.help-block { + &.form-text.text-muted { margin-left: 0; right: 0; } @@ -962,7 +962,7 @@ height: 30px; } - .help-block { + .form-text.text-muted { margin-top: 2px; color: $blue-500; cursor: pointer; @@ -1098,10 +1098,6 @@ font-size: 12px; } -.ide-new-modal-label { - line-height: 34px; -} - .multi-file-commit-panel-success-message { position: absolute; top: 61px; diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index ac71f72e624..9f5ad23a20f 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -93,8 +93,6 @@ class ProfilesController < Profiles::ApplicationController :linkedin, :location, :name, - :password, - :password_confirmation, :public_email, :skype, :twitter, diff --git a/app/controllers/projects/clusters_controller.rb b/app/controllers/projects/clusters_controller.rb index aeaba3a0acf..d58039b7d42 100644 --- a/app/controllers/projects/clusters_controller.rb +++ b/app/controllers/projects/clusters_controller.rb @@ -71,19 +71,6 @@ class Projects::ClustersController < Projects::ApplicationController .present(current_user: current_user) end - def create_params - params.require(:cluster).permit( - :enabled, - :name, - :provider_type, - provider_gcp_attributes: [ - :gcp_project_id, - :zone, - :num_nodes, - :machine_type - ]) - end - def update_params if cluster.managed? params.require(:cluster).permit( diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 52d528e816e..0821362f5df 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -7,6 +7,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController before_action :authorize_admin_environment!, only: [:terminal, :terminal_websocket_authorize] before_action :environment, only: [:show, :edit, :update, :stop, :terminal, :terminal_websocket_authorize, :metrics] before_action :verify_api_request!, only: :terminal_websocket_authorize + before_action :expire_etag_cache, only: [:index] def index @environments = project.environments @@ -148,6 +149,15 @@ class Projects::EnvironmentsController < Projects::ApplicationController Gitlab::Workhorse.verify_api_request!(request.headers) end + def expire_etag_cache + return if request.format.json? + + # this forces to reload json content + Gitlab::EtagCaching::Store.new.tap do |store| + store.touch(project_environments_path(project, format: :json)) + end + end + def environment_params params.require(:environment).permit(:name, :external_url) end diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb index 67d4ea2ca8f..29632bef7e5 100644 --- a/app/controllers/projects/merge_requests/application_controller.rb +++ b/app/controllers/projects/merge_requests/application_controller.rb @@ -24,6 +24,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont :source_branch, :source_project_id, :state_event, + :squash, :target_branch, :target_project_id, :task_num, diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 62b739918e6..507a07c6e1b 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -253,7 +253,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo end def merge_params_attributes - [:should_remove_source_branch, :commit_message] + [:should_remove_source_branch, :commit_message, :squash] end def merge_when_pipeline_succeeds_active? @@ -282,7 +282,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo return :sha_mismatch if params[:sha] != @merge_request.diff_head_sha - @merge_request.update(merge_error: nil) + @merge_request.update(merge_error: nil, squash: merge_params.fetch(:squash, false)) if params[:merge_when_pipeline_succeeds].present? return :failed unless @merge_request.actual_head_pipeline diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index b948e431882..adc423af9e1 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -204,7 +204,7 @@ module ApplicationSettingsHelper :pages_domain_verification_enabled, :password_authentication_enabled_for_web, :password_authentication_enabled_for_git, - :performance_bar_allowed_group_id, + :performance_bar_allowed_group_path, :performance_bar_enabled, :plantuml_enabled, :plantuml_url, diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index c19c5b9cc82..74251c260f0 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -97,8 +97,9 @@ module MergeRequestsHelper { merge_when_pipeline_succeeds: true, should_remove_source_branch: true, - sha: merge_request.diff_head_sha - }.merge(merge_params_ee(merge_request)) + sha: merge_request.diff_head_sha, + squash: merge_request.squash + } end def tab_link_for(merge_request, tab, options = {}, &block) @@ -149,8 +150,4 @@ module MergeRequestsHelper current_user.fork_of(project) end end - - def merge_params_ee(merge_request) - {} - end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index e8ccb320fae..b12f7a2c83f 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -230,6 +230,7 @@ class ApplicationSetting < ActiveRecord::Base after_commit do reset_memoized_terms end + after_commit :expire_performance_bar_allowed_user_ids_cache, if: -> { previous_changes.key?('performance_bar_allowed_group_id') } def self.defaults { @@ -386,31 +387,6 @@ class ApplicationSetting < ActiveRecord::Base super(levels.map { |level| Gitlab::VisibilityLevel.level_value(level) }) end - def performance_bar_allowed_group_id=(group_full_path) - group_full_path = nil if group_full_path.blank? - - if group_full_path.nil? - if group_full_path != performance_bar_allowed_group_id - super(group_full_path) - Gitlab::PerformanceBar.expire_allowed_user_ids_cache - end - - return - end - - group = Group.find_by_full_path(group_full_path) - - if group - if group.id != performance_bar_allowed_group_id - super(group.id) - Gitlab::PerformanceBar.expire_allowed_user_ids_cache - end - else - super(nil) - Gitlab::PerformanceBar.expire_allowed_user_ids_cache - end - end - def performance_bar_allowed_group Group.find_by_id(performance_bar_allowed_group_id) end @@ -420,15 +396,6 @@ class ApplicationSetting < ActiveRecord::Base performance_bar_allowed_group_id.present? end - # - If `enable` is true, we early return since the actual attribute that holds - # the enabling/disabling is `performance_bar_allowed_group_id` - # - If `enable` is false, we set `performance_bar_allowed_group_id` to `nil` - def performance_bar_enabled=(enable) - return if Gitlab::Utils.to_boolean(enable) - - self.performance_bar_allowed_group_id = nil - end - # Choose one of the available repository storage options. Currently all have # equal weighting. def pick_repository_storage @@ -506,4 +473,8 @@ class ApplicationSetting < ActiveRecord::Base errors.add(:terms, "You need to set terms to be enforced") unless terms.present? end + + def expire_performance_bar_allowed_user_ids_cache + Gitlab::PerformanceBar.expire_allowed_user_ids_cache + end end diff --git a/app/models/concerns/batch_destroy_dependent_associations.rb b/app/models/concerns/batch_destroy_dependent_associations.rb new file mode 100644 index 00000000000..353ee2e73d0 --- /dev/null +++ b/app/models/concerns/batch_destroy_dependent_associations.rb @@ -0,0 +1,28 @@ +# Provides a way to work around Rails issue where dependent objects are all +# loaded into memory before destroyed: https://github.com/rails/rails/issues/22510. +# +# This concern allows an ActiveRecord module to destroy all its dependent +# associations in batches. The idea is borrowed from https://github.com/thisismydesign/batch_dependent_associations. +# +# The differences here with that gem: +# +# 1. We allow excluding certain associations. +# 2. We don't need to support delete_all since we can use the EachBatch concern. +module BatchDestroyDependentAssociations + extend ActiveSupport::Concern + + DEPENDENT_ASSOCIATIONS_BATCH_SIZE = 1000 + + def dependent_associations_to_destroy + self.class.reflect_on_all_associations(:has_many).select { |assoc| assoc.options[:dependent] == :destroy } + end + + def destroy_dependent_associations_in_batches(exclude: []) + dependent_associations_to_destroy.each do |association| + next if exclude.include?(association.name) + + # rubocop:disable GitlabSecurity/PublicSend + public_send(association.name).find_each(batch_size: DEPENDENT_ASSOCIATIONS_BATCH_SIZE, &:destroy) + end + end +end diff --git a/app/models/event.rb b/app/models/event.rb index 741a84194e2..ac0b1c7b27c 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -40,6 +40,7 @@ class Event < ActiveRecord::Base ).freeze RESET_PROJECT_ACTIVITY_INTERVAL = 1.hour + REPOSITORY_UPDATED_AT_INTERVAL = 5.minutes delegate :name, :email, :public_email, :username, to: :author, prefix: true, allow_nil: true delegate :title, to: :issue, prefix: true, allow_nil: true @@ -391,6 +392,7 @@ class Event < ActiveRecord::Base def set_last_repository_updated_at Project.unscoped.where(id: project_id) + .where("last_repository_updated_at < ? OR last_repository_updated_at IS NULL", REPOSITORY_UPDATED_AT_INTERVAL.ago) .update_all(last_repository_updated_at: created_at) end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 9c4384a6e42..bc97fc3a5d9 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1140,4 +1140,11 @@ class MergeRequest < ActiveRecord::Base maintainer_push_possible? && Ability.allowed?(user, :push_code, source_project) end + + def squash_in_progress? + # The source project can be deleted + return false unless source_project + + source_project.repository.squash_in_progress?(id) + end end diff --git a/app/models/project.rb b/app/models/project.rb index 0fe9f8880b4..e275ac4dc6f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -24,6 +24,7 @@ class Project < ActiveRecord::Base include ChronicDurationAttribute include FastDestroyAll::Helpers include WithUploads + include BatchDestroyDependentAssociations extend Gitlab::ConfigHelper diff --git a/app/models/repository.rb b/app/models/repository.rb index 0e1bf11d7c0..6165808cd9a 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -957,6 +957,14 @@ class Repository remote_branch: merge_request.target_branch) end + def squash(user, merge_request) + raw.squash(user, merge_request.id, branch: merge_request.target_branch, + start_sha: merge_request.diff_start_sha, + end_sha: merge_request.diff_head_sha, + author: merge_request.author, + message: merge_request.title) + end + private # TODO Generice finder, later split this on finders by Ref or Oid diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index d0165c148eb..141070aef45 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -10,6 +10,7 @@ class MergeRequestWidgetEntity < IssuableEntity expose :merge_when_pipeline_succeeds expose :source_branch expose :source_project_id + expose :squash expose :target_branch expose :target_project_id expose :allow_maintainer_to_push diff --git a/app/services/application_settings/update_service.rb b/app/services/application_settings/update_service.rb index d6d3a661dab..e70445cfb67 100644 --- a/app/services/application_settings/update_service.rb +++ b/app/services/application_settings/update_service.rb @@ -3,6 +3,10 @@ module ApplicationSettings def execute update_terms(@params.delete(:terms)) + if params.key?(:performance_bar_allowed_group_path) + params[:performance_bar_allowed_group_id] = performance_bar_allowed_group_id + end + @application_setting.update(@params) end @@ -18,5 +22,13 @@ module ApplicationSettings ApplicationSetting::Term.create(terms: terms) @application_setting.reset_memoized_terms end + + def performance_bar_allowed_group_id + performance_bar_enabled = !params.key?(:performance_bar_enabled) || params.delete(:performance_bar_enabled) + group_full_path = params.delete(:performance_bar_allowed_group_path) + return nil unless Gitlab::Utils.to_boolean(performance_bar_enabled) + + Group.find_by_full_path(group_full_path)&.id if group_full_path.present? + end end end diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 2209a60a840..126da891c78 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -34,6 +34,19 @@ module MergeRequests handle_merge_error(log_message: e.message, save_message_on_model: true) end + def source + return merge_request.diff_head_sha unless merge_request.squash + + squash_result = ::MergeRequests::SquashService.new(project, current_user, params).execute(merge_request) + + case squash_result[:status] + when :success + squash_result[:squash_sha] + when :error + raise ::MergeRequests::MergeService::MergeError, squash_result[:message] + end + end + private def error_check! @@ -116,9 +129,5 @@ module MergeRequests def merge_request_info merge_request.to_reference(full: true) end - - def source - @source ||= @merge_request.diff_head_sha - end end end diff --git a/app/services/merge_requests/squash_service.rb b/app/services/merge_requests/squash_service.rb new file mode 100644 index 00000000000..a40fb2786bd --- /dev/null +++ b/app/services/merge_requests/squash_service.rb @@ -0,0 +1,28 @@ +module MergeRequests + class SquashService < MergeRequests::WorkingCopyBaseService + def execute(merge_request) + @merge_request = merge_request + @repository = target_project.repository + + squash || error('Failed to squash. Should be done manually.') + end + + def squash + if merge_request.commits_count < 2 + return success(squash_sha: merge_request.diff_head_sha) + end + + if merge_request.squash_in_progress? + return error('Squash task canceled: another squash is already in progress.') + end + + squash_sha = repository.squash(current_user, merge_request) + + success(squash_sha: squash_sha) + rescue => e + log_error("Failed to squash merge request #{merge_request.to_reference(full: true)}:") + log_error(e.message) + false + end + end +end diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 077d27c5836..de0125ed0dd 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -137,7 +137,13 @@ module Projects trash_repositories! - project.team.truncate + # Rails attempts to load all related records into memory before + # destroying: https://github.com/rails/rails/issues/22510 + # This ensures we delete records in batches. + # + # Exclude container repositories because its before_destroy would be + # called multiple times, and it doesn't destroy any database records. + project.destroy_dependent_associations_in_batches(exclude: [:container_repositories]) project.destroy! end end diff --git a/app/views/admin/application_settings/_performance_bar.html.haml b/app/views/admin/application_settings/_performance_bar.html.haml index 8001b42e2f9..030e8610b47 100644 --- a/app/views/admin/application_settings/_performance_bar.html.haml +++ b/app/views/admin/application_settings/_performance_bar.html.haml @@ -9,8 +9,8 @@ = f.check_box :performance_bar_enabled Enable the Performance Bar .form-group.row - = f.label :performance_bar_allowed_group_id, 'Allowed group', class: 'col-form-label col-sm-2' + = f.label :performance_bar_allowed_group_path, 'Allowed group', class: 'col-form-label col-sm-2' .col-sm-10 - = f.text_field :performance_bar_allowed_group_id, class: 'form-control', placeholder: 'my-org/my-group', value: @application_setting.performance_bar_allowed_group&.full_path + = f.text_field :performance_bar_allowed_group_path, class: 'form-control', placeholder: 'my-org/my-group', value: @application_setting.performance_bar_allowed_group&.full_path = f.submit 'Save changes', class: "btn btn-success" diff --git a/app/views/admin/application_settings/_repository_mirrors_form.html.haml b/app/views/admin/application_settings/_repository_mirrors_form.html.haml index 9d05a5aa234..edd8e5e9eb8 100644 --- a/app/views/admin/application_settings/_repository_mirrors_form.html.haml +++ b/app/views/admin/application_settings/_repository_mirrors_form.html.haml @@ -9,7 +9,7 @@ = f.label :mirror_available do = f.check_box :mirror_available Allow mirrors to be setup for projects - %span.help-block + %span.form-text.text-muted If disabled, only admins will be able to setup mirrors in projects. = link_to icon('question-circle'), help_page_path('workflow/repository_mirroring') diff --git a/app/views/admin/application_settings/_signin.html.haml b/app/views/admin/application_settings/_signin.html.haml index 4d74568d69a..83a30504222 100644 --- a/app/views/admin/application_settings/_signin.html.haml +++ b/app/views/admin/application_settings/_signin.html.haml @@ -23,7 +23,7 @@ must be used to authenticate. - if omniauth_enabled? && button_based_providers.any? .form-group.row - = f.label :enabled_oauth_sign_in_sources, 'Enabled OAuth sign-in sources', class: 'control-label col-sm-2' + = f.label :enabled_oauth_sign_in_sources, 'Enabled OAuth sign-in sources', class: 'col-form-label col-sm-2' = hidden_field_tag 'application_setting[enabled_oauth_sign_in_sources][]' .col-sm-10 .btn-group{ data: { toggle: 'buttons' } } diff --git a/app/views/admin/application_settings/_terms.html.haml b/app/views/admin/application_settings/_terms.html.haml index 32b060972ec..44bf7b65a8e 100644 --- a/app/views/admin/application_settings/_terms.html.haml +++ b/app/views/admin/application_settings/_terms.html.haml @@ -8,7 +8,7 @@ = f.label :enforce_terms do = f.check_box :enforce_terms = _("Require all users to accept Terms of Service when they access GitLab.") - .help-block + .form-text.text-muted = _("When enabled, users cannot use GitLab until the terms have been accepted.") .form-group .col-sm-12 @@ -16,7 +16,7 @@ = _("Terms of Service Agreement") .col-sm-12 = f.text_area :terms, class: 'form-control', rows: 8 - .help-block + .form-text.text-muted = _("Markdown enabled") = f.submit _("Save changes"), class: "btn btn-success" diff --git a/app/views/admin/groups/show.html.haml b/app/views/admin/groups/show.html.haml index 5ec612d0c72..6d75ccd5add 100644 --- a/app/views/admin/groups/show.html.haml +++ b/app/views/admin/groups/show.html.haml @@ -13,7 +13,7 @@ .card .card-header Group info: - %ul.well-list + %ul.content-list %li .avatar-container.s60 = group_icon(@group, class: "avatar s60") @@ -64,7 +64,7 @@ Projects %span.badge.badge-pill #{@group.projects.count} - %ul.well-list + %ul.content-list - @projects.each do |project| %li %strong @@ -82,7 +82,7 @@ Projects shared with #{@group.name} %span.badge.badge-pill #{@group.shared_projects.count} - %ul.well-list + %ul.content-list - @group.shared_projects.sort_by(&:name).each do |project| %li %strong @@ -118,7 +118,7 @@ %span.badge.badge-pill= @group.members.size .float-right = link_to icon('pencil-square-o', text: 'Manage access'), polymorphic_url([@group, :members]), class: "btn btn-sm" - %ul.well-list.group-users-list.content-list.members-list + %ul.content-list.group-users-list.content-list.members-list = render partial: 'shared/members/member', collection: @members, as: :member, locals: { show_controls: false } .card-footer = paginate @members, param_name: 'members_page', theme: 'gitlab' diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml index 29ec712b6b7..0a22a142858 100644 --- a/app/views/admin/projects/show.html.haml +++ b/app/views/admin/projects/show.html.haml @@ -22,7 +22,7 @@ .card .card-header Project info: - %ul.well-list + %ul.content-list %li %span.light Name: %strong @@ -166,7 +166,7 @@ .float-right = link_to admin_group_path(@group), class: 'btn btn-sm' do = icon('pencil-square-o', text: 'Manage access') - %ul.well-list.content-list.members-list + %ul.content-list.members-list = render partial: 'shared/members/member', collection: @group_members, as: :member, locals: { show_controls: false } .card-footer = paginate @group_members, param_name: 'group_members_page', theme: 'gitlab' @@ -180,7 +180,7 @@ %span.badge.badge-pill= @project.users.size .float-right = link_to icon('pencil-square-o', text: 'Manage access'), polymorphic_url([@project, :members]), class: "btn btn-sm" - %ul.well-list.project_members.content-list.members-list + %ul.content-list.project_members.members-list = render partial: 'shared/members/member', collection: @project_members, as: :member, locals: { show_controls: false } .card-footer = paginate @project_members, param_name: 'project_members_page', theme: 'gitlab' diff --git a/app/views/admin/users/_profile.html.haml b/app/views/admin/users/_profile.html.haml index af22652e07c..4fcb9aad343 100644 --- a/app/views/admin/users/_profile.html.haml +++ b/app/views/admin/users/_profile.html.haml @@ -1,7 +1,7 @@ .card .card-header Profile - %ul.well-list + %ul.content-list %li %span.light Member since %strong= user.created_at.to_s(:medium) diff --git a/app/views/admin/users/projects.html.haml b/app/views/admin/users/projects.html.haml index 469a7bd9715..cf50d45f755 100644 --- a/app/views/admin/users/projects.html.haml +++ b/app/views/admin/users/projects.html.haml @@ -4,7 +4,7 @@ - if @user.groups.any? .card .card-header Group projects - %ul.card-body-list + %ul.hover-list - @user.group_members.includes(:source).each do |group_member| - group = group_member.group %li.group_member @@ -28,7 +28,7 @@ .col-md-6 .card .card-header Joined projects (#{@joined_projects.count}) - %ul.card-body-list + %ul.hover-list - @joined_projects.sort_by(&:full_name).each do |project| - member = project.team.find_member(@user.id) %li.project_member diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml index a74fcea65d8..b0562226f5f 100644 --- a/app/views/admin/users/show.html.haml +++ b/app/views/admin/users/show.html.haml @@ -8,7 +8,7 @@ .card .card-header = @user.name - %ul.well-list + %ul.content-list %li = image_tag avatar_icon_for_user(@user, 60), class: "avatar s60" %li @@ -21,7 +21,7 @@ .card .card-header Account: - %ul.well-list + %ul.content-list %li %span.light Name: %strong= @user.name diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml index f85f5c5be88..85f2d00bde3 100644 --- a/app/views/events/event/_push.html.haml +++ b/app/views/events/event/_push.html.haml @@ -14,7 +14,7 @@ - if event.push_with_commits? .event-body - %ul.well-list.event_commits + %ul.content-list.event_commits = render "events/commit", project: project, event: event - create_mr = event.new_ref? && create_mr_button?(project.default_branch, event.ref_name, project) && event.authored_by?(current_user) diff --git a/app/views/groups/projects.html.haml b/app/views/groups/projects.html.haml index cd07b95155c..ba186875a86 100644 --- a/app/views/groups/projects.html.haml +++ b/app/views/groups/projects.html.haml @@ -8,7 +8,7 @@ .controls = link_to new_project_path(namespace_id: @group.id), class: "btn btn-sm btn-success" do New project - %ul.well-list + %ul.content-list - @projects.each do |project| %li .list-item-name diff --git a/app/views/groups/runners/_runner.html.haml b/app/views/groups/runners/_runner.html.haml index 76650a961d6..3f89b04a5fc 100644 --- a/app/views/groups/runners/_runner.html.haml +++ b/app/views/groups/runners/_runner.html.haml @@ -8,13 +8,13 @@ = link_to edit_group_runner_path(@group, runner) do = icon('edit') - .pull-right + .float-right - if runner.active? = link_to _('Pause'), pause_group_runner_path(@group, runner), method: :post, class: 'btn btn-sm btn-danger', data: { confirm: _("Are you sure?") } - else = link_to _('Resume'), resume_group_runner_path(@group, runner), method: :post, class: 'btn btn-success btn-sm' = link_to _('Remove Runner'), group_runner_path(@group, runner), data: { confirm: _("Are you sure?") }, method: :delete, class: 'btn btn-danger btn-sm' - .pull-right + .float-right %small.light \##{runner.id} - if runner.description.present? diff --git a/app/views/help/index.html.haml b/app/views/help/index.html.haml index 6391a13dd25..7a66bac09cb 100644 --- a/app/views/help/index.html.haml +++ b/app/views/help/index.html.haml @@ -36,7 +36,7 @@ .card .card-header Quick help - %ul.well-list + %ul.content-list %li= link_to 'See our website for getting help', support_url %li %button.btn-blank.btn-link.js-trigger-search-bar{ type: 'button' } diff --git a/app/views/help/ui.html.haml b/app/views/help/ui.html.haml index 94b012d39a3..a06db85ef6f 100644 --- a/app/views/help/ui.html.haml +++ b/app/views/help/ui.html.haml @@ -116,9 +116,9 @@ .lead List with hover effect - %code .well-list + %code .hover-list .example - %ul.well-list + %ul.hover-list %li One item %li @@ -131,7 +131,7 @@ .example .card .card-header Your list - %ul.well-list + %ul.content-list %li One item %li diff --git a/app/views/profiles/_event_table.html.haml b/app/views/profiles/_event_table.html.haml index 37466f7c821..9f525547dd9 100644 --- a/app/views/profiles/_event_table.html.haml +++ b/app/views/profiles/_event_table.html.haml @@ -1,7 +1,7 @@ %h5.prepend-top-0 History of authentications -%ul.well-list +%ul.content-list - events.each do |event| %li %span.description diff --git a/app/views/profiles/active_sessions/_active_session.html.haml b/app/views/profiles/active_sessions/_active_session.html.haml index d198bfc80db..23ef31a0c85 100644 --- a/app/views/profiles/active_sessions/_active_session.html.haml +++ b/app/views/profiles/active_sessions/_active_session.html.haml @@ -1,10 +1,10 @@ - is_current_session = active_session.current?(session) %li.list-group-item - .pull-left.append-right-10{ data: { toggle: 'tooltip' }, title: active_session.human_device_type } + .float-left.append-right-10{ data: { toggle: 'tooltip' }, title: active_session.human_device_type } = active_session_device_type_icon(active_session) - .description.pull-left + .description.float-left %div %strong= active_session.ip_address - if is_current_session @@ -25,7 +25,7 @@ = l(active_session.created_at, format: :short) - unless is_current_session - .pull-right + .float-right = link_to profile_active_session_path(active_session.session_id), data: { confirm: 'Are you sure? The device will be signed out of GitLab.' }, method: :delete, class: "btn btn-danger prepend-left-10" do %span.sr-only Revoke Revoke diff --git a/app/views/profiles/emails/index.html.haml b/app/views/profiles/emails/index.html.haml index 914bd4eb57c..a5db9dbe7f8 100644 --- a/app/views/profiles/emails/index.html.haml +++ b/app/views/profiles/emails/index.html.haml @@ -29,7 +29,7 @@ Your Public Email will be displayed on your public profile. %li All email addresses will be used to identify your commits. - %ul.well-list + %ul.content-list %li = render partial: 'shared/email_with_badge', locals: { email: @primary_email, verified: current_user.confirmed? } %span.float-right diff --git a/app/views/profiles/gpg_keys/_key_table.html.haml b/app/views/profiles/gpg_keys/_key_table.html.haml index cabb92c5a24..b9b60c218fd 100644 --- a/app/views/profiles/gpg_keys/_key_table.html.haml +++ b/app/views/profiles/gpg_keys/_key_table.html.haml @@ -1,7 +1,7 @@ - is_admin = local_assigns.fetch(:admin, false) - if @gpg_keys.any? - %ul.well-list + %ul.content-list = render partial: 'profiles/gpg_keys/key', collection: @gpg_keys, locals: { is_admin: is_admin } - else %p.settings-message.text-center diff --git a/app/views/profiles/keys/_key_details.html.haml b/app/views/profiles/keys/_key_details.html.haml index 02d5b08f7a3..2ac514d3f6f 100644 --- a/app/views/profiles/keys/_key_details.html.haml +++ b/app/views/profiles/keys/_key_details.html.haml @@ -4,7 +4,7 @@ .card .card-header SSH Key - %ul.well-list + %ul.content-list %li %span.light Title: %strong= @key.title diff --git a/app/views/profiles/keys/_key_table.html.haml b/app/views/profiles/keys/_key_table.html.haml index e78763bdcb2..e088140fdd2 100644 --- a/app/views/profiles/keys/_key_table.html.haml +++ b/app/views/profiles/keys/_key_table.html.haml @@ -1,7 +1,7 @@ - is_admin = local_assigns.fetch(:admin, false) - if @keys.any? - %ul.well-list + %ul.content-list = render partial: 'profiles/keys/key', collection: @keys, locals: { is_admin: is_admin } - else %p.settings-message.text-center diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 5b1f2d8953b..f641d7bc51a 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -28,7 +28,7 @@ = s_('Branches|Cant find HEAD commit for this branch') - if branch.name != @repository.root_ref - .divergence-graph.d-none.d-sm-block{ title: s_('%{number_commits_behind} commits behind %{default_branch}, %{number_commits_ahead} commits ahead') % { number_commits_behind: diverging_count_label(number_commits_behind), + .divergence-graph.d-none.d-md-block{ title: s_('%{number_commits_behind} commits behind %{default_branch}, %{number_commits_ahead} commits ahead') % { number_commits_behind: diverging_count_label(number_commits_behind), default_branch: @repository.root_ref, number_commits_ahead: diverging_count_label(number_commits_ahead) } } .graph-side @@ -39,7 +39,7 @@ .bar.bar-ahead{ style: "width: #{number_commits_ahead * bar_graph_width_factor}%" } %span.count.count-ahead= diverging_count_label(number_commits_ahead) - .controls.d-none.d-sm-block< + .controls.d-none.d-md-block< - if merge_project && create_mr_button?(@repository.root_ref, branch.name) = link_to create_mr_path(@repository.root_ref, branch.name), class: 'btn btn-default' do = _('Merge request') diff --git a/app/views/projects/clusters/gcp/_form.html.haml b/app/views/projects/clusters/gcp/_form.html.haml index be58a844f70..ca7a6d5a886 100644 --- a/app/views/projects/clusters/gcp/_form.html.haml +++ b/app/views/projects/clusters/gcp/_form.html.haml @@ -23,7 +23,7 @@ %span.dropdown-toggle-text = _('Select project') = icon('chevron-down') - %span.help-block + %span.form-text.text-muted .form-group = provider_gcp_field.label :zone, s_('ClusterIntegration|Zone') diff --git a/app/views/projects/hooks/_index.html.haml b/app/views/projects/hooks/_index.html.haml index 776681ea09a..5990582fd55 100644 --- a/app/views/projects/hooks/_index.html.haml +++ b/app/views/projects/hooks/_index.html.haml @@ -15,7 +15,7 @@ %h5.prepend-top-default Webhooks (#{@hooks.count}) - if @hooks.any? - %ul.well-list + %ul.content-list - @hooks.each do |hook| = render 'project_hook', hook: hook - else diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml index 9c9e4ef8fce..459150c1067 100644 --- a/app/views/projects/jobs/_sidebar.html.haml +++ b/app/views/projects/jobs/_sidebar.html.haml @@ -18,7 +18,7 @@ %span= time_ago_with_tooltip @build.artifacts_expire_at - if @build.artifacts? - .btn-group.btn-group.d-flex{ role: :group } + .btn-group.d-flex{ role: :group } - if @build.has_expiring_artifacts? && can?(current_user, :update_build, @build) = link_to keep_project_job_artifacts_path(@project, @build), class: 'btn btn-sm btn-default', method: :post do Keep @@ -42,7 +42,7 @@ - if @build.trigger_variables.any? %p - %button.btn.group.btn-group.js-reveal-variables Reveal Variables + %button.btn.group.js-reveal-variables Reveal Variables %dl.js-build-variables.trigger-build-variables.hide - @build.trigger_variables.each do |trigger_variable| diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index aa3fb623e58..01e38ffee20 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -20,6 +20,8 @@ window.gl = window.gl || {}; window.gl.mrWidgetData = #{serialize_issuable(@merge_request, serializer: 'widget')} + window.gl.mrWidgetData.squash_before_merge_help_path = '#{help_page_path("user/project/merge_requests/squash_and_merge")}'; + #js-vue-mr-widget.mr-widget .content-block.content-block-small.emoji-list-container.js-noteable-awards diff --git a/app/views/projects/mirrors/_push.html.haml b/app/views/projects/mirrors/_push.html.haml index 4a6aefce351..c3dcd9617a6 100644 --- a/app/views/projects/mirrors/_push.html.haml +++ b/app/views/projects/mirrors/_push.html.haml @@ -30,7 +30,7 @@ #{h(@remote_mirror.last_error.strip)} = f.fields_for :remote_mirrors, @remote_mirror do |rm_form| .form-group - = rm_form.check_box :enabled, class: "pull-left" + = rm_form.check_box :enabled, class: "float-left" .prepend-left-20 = rm_form.label :enabled, "Remote mirror repository", class: "label-light append-bottom-0" %p.light.append-bottom-0 @@ -42,7 +42,7 @@ = render "projects/mirrors/instructions" .form-group - = rm_form.check_box :only_protected_branches, class: 'pull-left' + = rm_form.check_box :only_protected_branches, class: 'float-left' .prepend-left-20 = rm_form.label :only_protected_branches, class: 'label-light' = link_to icon('question-circle'), help_page_path('user/project/protected_branches') diff --git a/app/views/projects/pages/_list.html.haml b/app/views/projects/pages/_list.html.haml index 986ca852411..e7178f9160c 100644 --- a/app/views/projects/pages/_list.html.haml +++ b/app/views/projects/pages/_list.html.haml @@ -4,7 +4,7 @@ .card .card-header Domains (#{@domains.count}) - %ul.well-list.pages-domain-list{ class: ("has-verification-status" if verification_enabled) } + %ul.content-list.pages-domain-list{ class: ("has-verification-status" if verification_enabled) } - @domains.each do |domain| %li.pages-domain-list-item.unstyled - if verification_enabled diff --git a/app/views/projects/pipelines/new.html.haml b/app/views/projects/pipelines/new.html.haml index d1e8e9d0d60..956f8fef6b8 100644 --- a/app/views/projects/pipelines/new.html.haml +++ b/app/views/projects/pipelines/new.html.haml @@ -29,7 +29,7 @@ .form-actions = f.submit s_('Pipeline|Create pipeline'), class: 'btn btn-success js-variables-save-button', tabindex: 3 - = link_to 'Cancel', project_pipelines_path(@project), class: 'btn btn-default pull-right' + = link_to 'Cancel', project_pipelines_path(@project), class: 'btn btn-default float-right' -# haml-lint:disable InlineJavaScript %script#availableRefs{ type: "application/json" }= @project.repository.ref_names.to_json.html_safe diff --git a/app/views/projects/settings/ci_cd/_autodevops_form.html.haml b/app/views/projects/settings/ci_cd/_autodevops_form.html.haml index 988bcfb5265..414df15feeb 100644 --- a/app/views/projects/settings/ci_cd/_autodevops_form.html.haml +++ b/app/views/projects/settings/ci_cd/_autodevops_form.html.haml @@ -34,7 +34,7 @@ = form.label :domain, class:"prepend-top-10" do = _('Domain') = form.text_field :domain, class: 'form-control', placeholder: 'domain.com' - .help-block + .form-text.text-muted = s_('CICD|A domain is required to use Auto Review Apps and Auto Deploy Stages.') - if cluster_ingress_ip = cluster_ingress_ip(@project) = s_('%{nip_domain} can be used as an alternative to a custom domain.').html_safe % { nip_domain: "<code>#{cluster_ingress_ip}.nip.io</code>".html_safe } diff --git a/app/views/shared/_email_with_badge.html.haml b/app/views/shared/_email_with_badge.html.haml index b7bbc109238..ad863b1967d 100644 --- a/app/views/shared/_email_with_badge.html.haml +++ b/app/views/shared/_email_with_badge.html.haml @@ -1,4 +1,4 @@ -- css_classes = %w(label label-verification-status) +- css_classes = %w(badge badge-verification-status) - css_classes << (verified ? 'verified': 'unverified') - text = verified ? 'Verified' : 'Unverified' diff --git a/app/views/shared/_visibility_level.html.haml b/app/views/shared/_visibility_level.html.haml index d67409ffe14..38c6f560dc6 100644 --- a/app/views/shared/_visibility_level.html.haml +++ b/app/views/shared/_visibility_level.html.haml @@ -1,6 +1,6 @@ - with_label = local_assigns.fetch(:with_label, true) -.form-group.visibility-level-setting +.form-group.row.visibility-level-setting - if with_label = f.label :visibility_level, class: 'col-form-label col-sm-2' do Visibility Level diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 602729b172a..a57cd4b20d1 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -33,7 +33,7 @@ = link_to _('Edit'), '#', class: 'js-sidebar-dropdown-toggle edit-link float-right' .value.hide-collapsed - if issuable.milestone - = link_to issuable.milestone.title, milestone_path(issuable.milestone), class: "bold has-tooltip", title: milestone_tooltip_due_date(issuable.milestone), data: { container: "body", html: 'true' } + = link_to issuable.milestone.title, milestone_path(issuable.milestone), class: "bold has-tooltip", title: milestone_tooltip_due_date(issuable.milestone), data: { container: "body", html: 'true', boundary: 'viewport' } - else %span.no-value = _('None') diff --git a/app/views/shared/issuable/form/_merge_params.html.haml b/app/views/shared/issuable/form/_merge_params.html.haml index 1df881e4102..90fbf19e843 100644 --- a/app/views/shared/issuable/form/_merge_params.html.haml +++ b/app/views/shared/issuable/form/_merge_params.html.haml @@ -15,3 +15,12 @@ = hidden_field_tag 'merge_request[force_remove_source_branch]', '0', id: nil = check_box_tag 'merge_request[force_remove_source_branch]', '1', issuable.force_remove_source_branch? Remove source branch when merge request is accepted. + +.form-group + .col-sm-10.col-sm-offset-2 + .checkbox + = label_tag 'merge_request[squash]' do + = hidden_field_tag 'merge_request[squash]', '0', id: nil + = check_box_tag 'merge_request[squash]', '1', issuable.squash + Squash commits when merge request is accepted. + = link_to 'About this feature', help_page_path('user/project/merge_requests/squash_and_merge') diff --git a/app/views/shared/milestones/_issuables.html.haml b/app/views/shared/milestones/_issuables.html.haml index d8e4d2ff88c..ee6354b1c28 100644 --- a/app/views/shared/milestones/_issuables.html.haml +++ b/app/views/shared/milestones/_issuables.html.haml @@ -11,7 +11,7 @@ = number_with_delimiter(issuables.length) - class_prefix = dom_class(issuables).pluralize - %ul{ class: "well-list milestone-#{class_prefix}-list", id: "#{class_prefix}-list-#{id}" } + %ul{ class: "content-list milestone-#{class_prefix}-list", id: "#{class_prefix}-list-#{id}" } = render partial: 'shared/milestones/issuable', collection: issuables, as: :issuable, diff --git a/app/views/shared/snippets/_snippet.html.haml b/app/views/shared/snippets/_snippet.html.haml index e036b21b23f..5069e2e4ca6 100644 --- a/app/views/shared/snippets/_snippet.html.haml +++ b/app/views/shared/snippets/_snippet.html.haml @@ -28,7 +28,7 @@ = link_to user_snippets_path(snippet.author) do = snippet.author_name - if link_project && snippet.project_id? - %span.d-none.d-sm-block + %span.d-none.d-sm-inline-block in = link_to project_path(snippet.project) do = snippet.project.full_name diff --git a/app/views/sherlock/queries/_backtrace.html.haml b/app/views/sherlock/queries/_backtrace.html.haml index 4f5146cefb9..38b4d2c6102 100644 --- a/app/views/sherlock/queries/_backtrace.html.haml +++ b/app/views/sherlock/queries/_backtrace.html.haml @@ -3,7 +3,7 @@ .card-header %strong = t('sherlock.application_backtrace') - %ul.well-list + %ul.content-list - @query.application_backtrace.each do |location| %li %strong @@ -19,7 +19,7 @@ .card-header %strong = t('sherlock.full_backtrace') - %ul.well-list + %ul.content-list - @query.backtrace.each do |location| %li - if location.application? diff --git a/app/views/sherlock/queries/_general.html.haml b/app/views/sherlock/queries/_general.html.haml index 34c0cc4da39..37747faed62 100644 --- a/app/views/sherlock/queries/_general.html.haml +++ b/app/views/sherlock/queries/_general.html.haml @@ -3,7 +3,7 @@ .card-header %strong = t('sherlock.general') - %ul.well-list + %ul.content-list %li %span.light #{t('sherlock.time')}: @@ -32,7 +32,7 @@ = @query.formatted_query %strong = t('sherlock.query') - %ul.well-list + %ul.content-list %li .code.js-syntax-highlight.sherlock-code :preserve @@ -47,7 +47,7 @@ = @query.explain %strong = t('sherlock.query_plan') - %ul.well-list + %ul.content-list %li .code.js-syntax-highlight.sherlock-code %pre diff --git a/app/views/sherlock/transactions/_general.html.haml b/app/views/sherlock/transactions/_general.html.haml index 7ec8dde8421..9c028b5c741 100644 --- a/app/views/sherlock/transactions/_general.html.haml +++ b/app/views/sherlock/transactions/_general.html.haml @@ -3,7 +3,7 @@ .card-header %strong = t('sherlock.general') - %ul.well-list + %ul.content-list %li %span.light #{t('sherlock.id')}: diff --git a/app/views/users/terms/index.html.haml b/app/views/users/terms/index.html.haml index c5406696bdd..e0fe551cf36 100644 --- a/app/views/users/terms/index.html.haml +++ b/app/views/users/terms/index.html.haml @@ -4,10 +4,10 @@ = markdown_field(@term, :terms) .row-content-block.footer-block.clearfix - if can?(current_user, :accept_terms, @term) - .pull-right + .float-right = button_to accept_term_path(@term, redirect_params), class: 'btn btn-success prepend-left-8' do = _('Accept terms') - if can?(current_user, :decline_terms, @term) - .pull-right + .float-right = button_to decline_term_path(@term, redirect_params), class: 'btn btn-default prepend-left-8' do = _('Decline and sign out') diff --git a/changelogs/unreleased/46844-update-awesome_print-to-1-8-0.yml b/changelogs/unreleased/46844-update-awesome_print-to-1-8-0.yml new file mode 100644 index 00000000000..e6dc9a6187b --- /dev/null +++ b/changelogs/unreleased/46844-update-awesome_print-to-1-8-0.yml @@ -0,0 +1,5 @@ +--- +title: Update awesome_print to 1.8.0 +merge_request: 19163 +author: Takuya Noguchi +type: other diff --git a/changelogs/unreleased/46849-update-rdoc-to-6-0-4.yml b/changelogs/unreleased/46849-update-rdoc-to-6-0-4.yml new file mode 100644 index 00000000000..cf0436df1a7 --- /dev/null +++ b/changelogs/unreleased/46849-update-rdoc-to-6-0-4.yml @@ -0,0 +1,5 @@ +--- +title: Update rdoc to 6.0.4 +merge_request: 19167 +author: Takuya Noguchi +type: other diff --git a/changelogs/unreleased/ab-35364-throttle-updates-last-repository-at.yml b/changelogs/unreleased/ab-35364-throttle-updates-last-repository-at.yml new file mode 100644 index 00000000000..8e468233637 --- /dev/null +++ b/changelogs/unreleased/ab-35364-throttle-updates-last-repository-at.yml @@ -0,0 +1,5 @@ +--- +title: Throttle updates to Project#last_repository_updated_at. +merge_request: 19183 +author: +type: performance diff --git a/changelogs/unreleased/blackst0ne-squash-and-merge-in-gitlab-core-ce.yml b/changelogs/unreleased/blackst0ne-squash-and-merge-in-gitlab-core-ce.yml new file mode 100644 index 00000000000..e603c835b5e --- /dev/null +++ b/changelogs/unreleased/blackst0ne-squash-and-merge-in-gitlab-core-ce.yml @@ -0,0 +1,5 @@ +--- +title: Add `Squash and merge` to GitLab Core (CE) +merge_request: 18956 +author: "@blackst0ne" +type: added diff --git a/changelogs/unreleased/security-dm-delete-deploy-key.yml b/changelogs/unreleased/security-dm-delete-deploy-key.yml new file mode 100644 index 00000000000..aa94e7b6072 --- /dev/null +++ b/changelogs/unreleased/security-dm-delete-deploy-key.yml @@ -0,0 +1,5 @@ +--- +title: Fix API to remove deploy key from project instead of deleting it entirely +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-fj-import-export-assignment.yml b/changelogs/unreleased/security-fj-import-export-assignment.yml new file mode 100644 index 00000000000..4bfd71d431a --- /dev/null +++ b/changelogs/unreleased/security-fj-import-export-assignment.yml @@ -0,0 +1,5 @@ +--- +title: Fixed bug that allowed importing arbitrary project attributes +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-users-can-update-their-password-without-entering-current-password.yml b/changelogs/unreleased/security-users-can-update-their-password-without-entering-current-password.yml new file mode 100644 index 00000000000..824fbd41ab8 --- /dev/null +++ b/changelogs/unreleased/security-users-can-update-their-password-without-entering-current-password.yml @@ -0,0 +1,5 @@ +--- +title: Prevent user passwords from being changed without providing the previous password +merge_request: +author: +type: security diff --git a/changelogs/unreleased/sh-batch-dependent-destroys.yml b/changelogs/unreleased/sh-batch-dependent-destroys.yml new file mode 100644 index 00000000000..e297badc1fa --- /dev/null +++ b/changelogs/unreleased/sh-batch-dependent-destroys.yml @@ -0,0 +1,5 @@ +--- +title: Fix project destruction failing due to idle in transaction timeouts +merge_request: +author: +type: fixed diff --git a/db/migrate/20180515005612_add_squash_to_merge_requests.rb b/db/migrate/20180515005612_add_squash_to_merge_requests.rb new file mode 100644 index 00000000000..f526b45bd4b --- /dev/null +++ b/db/migrate/20180515005612_add_squash_to_merge_requests.rb @@ -0,0 +1,19 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddSquashToMergeRequests < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + + DOWNTIME = false + + def up + unless column_exists?(:merge_requests, :squash) + add_column_with_default :merge_requests, :squash, :boolean, default: false, allow_null: false + end + end + + def down + remove_column :merge_requests, :squash if column_exists?(:merge_requests, :squash) + end +end diff --git a/db/schema.rb b/db/schema.rb index 932b7f8da02..42fea8e4380 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1217,6 +1217,7 @@ ActiveRecord::Schema.define(version: 20180529093006) do t.integer "latest_merge_request_diff_id" t.string "rebase_commit_sha" t.boolean "allow_maintainer_to_push" + t.boolean "squash", default: false, null: false end add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 4e34831422a..8849f490c4f 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -107,6 +107,7 @@ Parameters: "changes_count": "1", "should_remove_source_branch": true, "force_remove_source_branch": false, + "squash": false, "web_url": "http://example.com/example/example/merge_requests/1", "time_stats": { "time_estimate": 0, @@ -226,6 +227,7 @@ Parameters: "changes_count": "1", "should_remove_source_branch": true, "force_remove_source_branch": false, + "squash": false, "web_url": "http://example.com/example/example/merge_requests/1", "discussion_locked": false, "time_stats": { @@ -305,6 +307,7 @@ Parameters: "changes_count": "1", "should_remove_source_branch": true, "force_remove_source_branch": false, + "squash": false, "web_url": "http://example.com/example/example/merge_requests/1", "discussion_locked": false, "time_stats": { @@ -541,7 +544,8 @@ POST /projects/:id/merge_requests | `labels` | string | no | Labels for MR as a comma-separated list | | `milestone_id` | integer | no | The global ID of a milestone | | `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging | -| `allow_maintainer_to_push` | boolean | no | Whether or not a maintainer of the target project can push to the source branch | +| `allow_maintainer_to_push` | boolean | no | Whether or not a maintainer of the target project can push to the source branch | +| `squash` | boolean | no | Squash commits into a single commit when merging | ```json { @@ -595,6 +599,7 @@ POST /projects/:id/merge_requests "changes_count": "1", "should_remove_source_branch": true, "force_remove_source_branch": false, + "squash": false, "web_url": "http://example.com/example/example/merge_requests/1", "discussion_locked": false, "allow_maintainer_to_push": false, @@ -627,6 +632,7 @@ PUT /projects/:id/merge_requests/:merge_request_iid | `description` | string | no | Description of MR | | `state_event` | string | no | New state (close/reopen) | | `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging | +| `squash` | boolean | no | Squash commits into a single commit when merging | | `discussion_locked` | boolean | no | Flag indicating if the merge request's discussion is locked. If the discussion is locked only project members can add, edit or resolve comments. | | `allow_maintainer_to_push` | boolean | no | Whether or not a maintainer of the target project can push to the source branch | @@ -683,6 +689,7 @@ Must include at least one non-required attribute from above. "changes_count": "1", "should_remove_source_branch": true, "force_remove_source_branch": false, + "squash": false, "web_url": "http://example.com/example/example/merge_requests/1", "discussion_locked": false, "allow_maintainer_to_push": false, @@ -790,6 +797,7 @@ Parameters: "changes_count": "1", "should_remove_source_branch": true, "force_remove_source_branch": false, + "squash": false, "web_url": "http://example.com/example/example/merge_requests/1", "discussion_locked": false, "time_stats": { @@ -868,6 +876,7 @@ Parameters: "changes_count": "1", "should_remove_source_branch": true, "force_remove_source_branch": false, + "squash": false, "web_url": "http://example.com/example/example/merge_requests/1", "discussion_locked": false, "time_stats": { @@ -1200,6 +1209,7 @@ Example response: "changes_count": "1", "should_remove_source_branch": true, "force_remove_source_branch": false, + "squash": false, "web_url": "http://example.com/example/example/merge_requests/1" }, "target_url": "https://gitlab.example.com/gitlab-org/gitlab-ci/merge_requests/7", diff --git a/doc/api/settings.md b/doc/api/settings.md index 1ebfe4924b1..36a0782d8f2 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -55,6 +55,7 @@ Example response: "ed25519_key_restriction": 0, "enforce_terms": true, "terms": "Hello world!", + "performance_bar_allowed_group_id": 42 } ``` @@ -120,8 +121,9 @@ PUT /application/settings | `metrics_timeout` | integer | yes (if `metrics_enabled` is `true`) | The amount of seconds after which InfluxDB will time out. | | `password_authentication_enabled_for_web` | boolean | no | Enable authentication for the web interface via a GitLab account password. Default is `true`. | | `password_authentication_enabled_for_git` | boolean | no | Enable authentication for Git over HTTP(S) via a GitLab account password. Default is `true`. | -| `performance_bar_allowed_group_id` | string | no | The group that is allowed to enable the performance bar | -| `performance_bar_enabled` | boolean | no | Allow enabling the performance bar | +| `performance_bar_allowed_group_path` | string | no | Path of the group that is allowed to toggle the performance bar | +| `performance_bar_allowed_group_id` | string | no | Deprecated: Use `performance_bar_allowed_group_path` instead. Path of the group that is allowed to toggle the performance bar | +| `performance_bar_enabled` | boolean | no | Deprecated: Pass `performance_bar_allowed_group_path: nil` instead. Allow enabling the performance bar | | `plantuml_enabled` | boolean | no | Enable PlantUML integration. Default is `false`. | | `plantuml_url` | string | yes (if `plantuml_enabled` is `true`) | The PlantUML instance URL for integration. | | `polling_interval_multiplier` | decimal | no | Interval multiplier used by endpoints that perform polling. Set to 0 to disable polling. | @@ -201,5 +203,6 @@ Example response: "ed25519_key_restriction": 0, "enforce_terms": true, "terms": "Hello world!", + "performance_bar_allowed_group_id": 42 } ``` diff --git a/doc/ci/README.md b/doc/ci/README.md index 8d1d72c2a2b..7666219acb0 100644 --- a/doc/ci/README.md +++ b/doc/ci/README.md @@ -19,7 +19,7 @@ Here's some info we've gathered to get you started. The first steps towards your GitLab CI/CD journey. - [Getting started with GitLab CI/CD](quick_start/README.md): understand how GitLab CI/CD works. -- GitLab CI/CD configuration file: [`.gitlab-ci.yml`](yaml/README.md) - Learn all about the ins and outs of `.gitlab-ci.yml`. +- [GitLab CI/CD configuration file: `.gitlab-ci.yml`](yaml/README.md) - Learn all about the ins and outs of `.gitlab-ci.yml`. - [Pipelines and jobs](pipelines.md): configure your GitLab CI/CD pipelines to build, test, and deploy your application. - Runners: The [GitLab Runner](https://docs.gitlab.com/runner/) is responsible by running the jobs in your CI/CD pipeline. On GitLab.com, Shared Runners are enabled by default, so you don't need to set up anything to start to use them with GitLab CI/CD. @@ -46,7 +46,9 @@ you don't need to set up anything to start to use them with GitLab CI/CD. ## Exploring GitLab CI/CD - [CI/CD Variables](variables/README.md) - Learn how to use variables defined in - your `.gitlab-ci.yml` or secured ones defined in your project's settings + your `.gitlab-ci.yml` or the ones defined in your project's settings + - [Where variables can be used](variables/where_variables_can_be_used.md) - A + deeper look on where and how the CI/CD variables can be used - **The permissions model** - Learn about the access levels a user can have for performing certain CI actions - [User permissions](../user/permissions.md#gitlab-ci) diff --git a/doc/ci/environments.md b/doc/ci/environments.md index 0d54f375c93..7f034409580 100644 --- a/doc/ci/environments.md +++ b/doc/ci/environments.md @@ -246,23 +246,14 @@ As the name suggests, it is possible to create environments on the fly by just declaring their names dynamically in `.gitlab-ci.yml`. Dynamic environments is the basis of [Review apps](review_apps/index.md). ->**Note:** -The `name` and `url` parameters can use most of the defined CI variables, -including predefined, secure variables and `.gitlab-ci.yml` -[`variables`](yaml/README.md#variables). You however cannot use variables -defined under `script` or on the Runner's side. There are other variables that -are unsupported in environment name context: -- `CI_PIPELINE_ID` -- `CI_JOB_ID` -- `CI_JOB_TOKEN` -- `CI_BUILD_ID` -- `CI_BUILD_TOKEN` -- `CI_REGISTRY_USER` -- `CI_REGISTRY_PASSWORD` -- `CI_REPOSITORY_URL` -- `CI_ENVIRONMENT_URL` -- `CI_DEPLOY_USER` -- `CI_DEPLOY_PASSWORD` +NOTE: **Note:** +The `name` and `url` parameters can use most of the CI/CD variables, +including [predefined](variables/README.md#predefined-variables-environment-variables), +[secret](variables/README.md#secret-variables) and +[`.gitlab-ci.yml` variables](yaml/README.md#variables). You however cannot use variables +defined under `script` or on the Runner's side. There are also other variables that +are unsupported in the context of `environment:name`. You can read more about +[where variables can be used](variables/where_variables_can_be_used.md). GitLab Runner exposes various [environment variables][variables] when a job runs, and as such, you can use them as environment names. Let's add another job in diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index 683846a536b..f10423b92cf 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -22,6 +22,12 @@ For example, if you define `API_TOKEN=secure` as a secret variable and `API_TOKEN=yaml` in your `.gitlab-ci.yml`, the `API_TOKEN` will take the value `secure` as the secret variables are higher in the chain. +## Unsupported variables + +There are cases where some variables cannot be used in the context of a +`.gitlab-ci.yml` definition (for example under `script`). Read more +about which variables are [not supported](where_variables_can_be_used.md). + ## Predefined variables (Environment variables) Some of the predefined environment variables are available only if a minimum @@ -36,6 +42,7 @@ future GitLab releases.** | Variable | GitLab | Runner | Description | |-------------------------------- |--------|--------|-------------| +| **ARTIFACT_DOWNLOAD_ATTEMPTS** | 8.15 | 1.9 | Number of attempts to download artifacts running a job | | **CI** | all | 0.4 | Mark that job is executed in CI environment | | **CI_COMMIT_REF_NAME** | 9.0 | all | The branch or tag name for which project is built | | **CI_COMMIT_REF_SLUG** | 9.0 | all | `$CI_COMMIT_REF_NAME` lowercased, shortened to 63 bytes, and with everything except `0-9` and `a-z` replaced with `-`. No leading / trailing `-`. Use in URLs, host names and domain names. | @@ -46,6 +53,8 @@ future GitLab releases.** | **CI_COMMIT_DESCRIPTION** | 10.8 | all | The description of the commit: the message without first line, if the title is shorter than 100 characters; full message in other case. | | **CI_CONFIG_PATH** | 9.4 | 0.5 | The path to CI config file. Defaults to `.gitlab-ci.yml` | | **CI_DEBUG_TRACE** | all | 1.7 | Whether [debug tracing](#debug-tracing) is enabled | +| **CI_DEPLOY_USER** | 10.8 | all | Authentication username of the [GitLab Deploy Token][gitlab-deploy-token], only present if the Project has one related.| +| **CI_DEPLOY_PASSWORD** | 10.8 | all | Authentication password of the [GitLab Deploy Token][gitlab-deploy-token], only present if the Project has one related.| | **CI_DISPOSABLE_ENVIRONMENT** | all | 10.1 | Marks that the job is executed in a disposable environment (something that is created only for this job and disposed of/destroyed after the execution - all executors except `shell` and `ssh`). If the environment is disposable, it is set to true, otherwise it is not defined at all. | | **CI_ENVIRONMENT_NAME** | 8.15 | all | The name of the environment for this job | | **CI_ENVIRONMENT_SLUG** | 8.15 | all | A simplified version of the environment name, suitable for inclusion in DNS, URLs, Kubernetes labels, etc. | @@ -82,16 +91,13 @@ future GitLab releases.** | **CI_SERVER_REVISION** | all | all | GitLab revision that is used to schedule jobs | | **CI_SERVER_VERSION** | all | all | GitLab version that is used to schedule jobs | | **CI_SHARED_ENVIRONMENT** | all | 10.1 | Marks that the job is executed in a shared environment (something that is persisted across CI invocations like `shell` or `ssh` executor). If the environment is shared, it is set to true, otherwise it is not defined at all. | -| **ARTIFACT_DOWNLOAD_ATTEMPTS** | 8.15 | 1.9 | Number of attempts to download artifacts running a job | | **GET_SOURCES_ATTEMPTS** | 8.15 | 1.9 | Number of attempts to fetch sources running a job | | **GITLAB_CI** | all | all | Mark that job is executed in GitLab CI environment | -| **GITLAB_USER_ID** | 8.12 | all | The id of the user who started the job | | **GITLAB_USER_EMAIL** | 8.12 | all | The email of the user who started the job | +| **GITLAB_USER_ID** | 8.12 | all | The id of the user who started the job | | **GITLAB_USER_LOGIN** | 10.0 | all | The login username of the user who started the job | | **GITLAB_USER_NAME** | 10.0 | all | The real name of the user who started the job | | **RESTORE_CACHE_ATTEMPTS** | 8.15 | 1.9 | Number of attempts to restore the cache running a job | -| **CI_DEPLOY_USER** | 10.8 | all | Authentication username of the [GitLab Deploy Token][gitlab-deploy-token], only present if the Project has one related.| -| **CI_DEPLOY_PASSWORD** | 10.8 | all | Authentication password of the [GitLab Deploy Token][gitlab-deploy-token], only present if the Project has one related.| ## 9.0 Renaming @@ -540,34 +546,6 @@ Below you can find supported syntax reference: Pattern matching is case-sensitive by default. Use `i` flag modifier, like `/pattern/i` to make a pattern case-insensitive. -### Unsupported predefined variables - -Because GitLab evaluates variables before creating jobs, we do not support a -few variables that depend on persistence layer, like `$CI_JOB_ID`. - -Environments (like `production` or `staging`) are also being created based on -what jobs pipeline consists of, thus some environment-specific variables are -not supported as well. - -We do not support variables containing tokens because of security reasons. - -You can find a full list of unsupported variables below: - -- `CI_PIPELINE_ID` -- `CI_JOB_ID` -- `CI_JOB_TOKEN` -- `CI_BUILD_ID` -- `CI_BUILD_TOKEN` -- `CI_REGISTRY_USER` -- `CI_REGISTRY_PASSWORD` -- `CI_REPOSITORY_URL` -- `CI_ENVIRONMENT_URL` -- `CI_DEPLOY_USER` -- `CI_DEPLOY_PASSWORD` - -These variables are also not supported in a context of a -[dynamic environment name][dynamic-environments]. - [ce-13784]: https://gitlab.com/gitlab-org/gitlab-ce/issues/13784 "Simple protection of CI secret variables" [eep]: https://about.gitlab.com/products/ "Available only in GitLab Premium" [envs]: ../environments.md @@ -579,5 +557,4 @@ These variables are also not supported in a context of a [triggers]: ../triggers/README.md#pass-job-variables-to-a-trigger [subgroups]: ../../user/group/subgroups/index.md [builds-policies]: ../yaml/README.md#only-and-except-complex -[dynamic-environments]: ../environments.md#dynamic-environments [gitlab-deploy-token]: ../../user/project/deploy_tokens/index.md#gitlab-deploy-token diff --git a/doc/ci/variables/where_variables_can_be_used.md b/doc/ci/variables/where_variables_can_be_used.md new file mode 100644 index 00000000000..9800784d918 --- /dev/null +++ b/doc/ci/variables/where_variables_can_be_used.md @@ -0,0 +1,113 @@ +# Where variables can be used + +As it's described in the [CI/CD variables](README.md) docs, you can +define many different variables. Some of them can be used for all GitLab CI/CD +features, but some of them are more or less limited. + +This document describes where and how the different types of variables can be used. + +## Variables usage + +There are basically two places where you can use any defined variables: + +1. On GitLab's side there's `.gitlab-ci.yml` +1. On the Runner's side there's `config.toml` + +### `.gitlab-ci.yml` file + +| Definition | Can be expanded? | Expansion place | Description | +|--------------------------------------|-------------------|-----------------|--------------| +| `environment:url` | yes | GitLab | The variable expansion is made by GitLab's [internal variable expansion mechanism](#gitlab-internal-variable-expansion-mechanism).<ul><li>**Supported:** all variables defined for a job (secret variables, variables from `.gitlab-ci.yml`, variables from triggers, variables from pipeline schedules)</li><li>**Not suported:** variables defined in Runner's `config.toml` and variables created in job's `script`</li></ul> | +| `environment:name` | yes | GitLab | Similar to `environment:url`, but the variables expansion **doesn't support**: <ul><li>variables that are based on the environment's name (`CI_ENVIRONMENT_NAME`, `CI_ENVIRONMENT_SLUG`)</li><li>any other variables related to environment (currently only `CI_ENVIRONMENT_URL`)</li><li>[persisted variables](#persisted-variables)</li></ul> | +| `variables` | yes | Runner | The variable expansion is made by GitLab Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism) | +| `image` | yes | Runner | The variable expansion is made by GitLab Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism) | +| `services:[]` | yes | Runner | The variable expansion is made by GitLab Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism) | +| `services:[]:name` | yes | Runner | The variable expansion is made by GitLab Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism) | +| `cache:key` | yes | Runner | The variable expansion is made by GitLab Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism) | +| `artifacts:name` | yes | Runner | The variable expansion is made by GitLab Runner's shell environment | +| `script`, `before_script`, `after_script` | yes | Script execution shell | The variable expansion is made by the [execution shell environment](#execution-shell-environment) | +| `only:variables:[]`, `except:variables:[]` | no | n/a | The variable must be in the form of `$variable`.<br/>**Not supported:**<ul><li>variables that are based on the environment's name (`CI_ENVIRONMENT_NAME`, `CI_ENVIRONMENT_SLUG`)</li><li>any other variables related to environment (currently only `CI_ENVIRONMENT_URL`)</li><li>[persisted variables](#persisted-variables)</li></ul> | + +### `config.toml` file + +NOTE: **Note:** +You can read more about `config.toml` in the [Runner's docs](https://docs.gitlab.com/runner/configuration/advanced-configuration.html). + +| Definition | Can be expanded? | Description | +|--------------------------------------|------------------|-------------| +| `runners.environment` | yes | The variable expansion is made by the Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism) | +| `runners.kubernetes.pod_labels` | yes | The Variable expansion is made by the Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism) | +| `runners.kubernetes.pod_annotations` | yes | The Variable expansion is made by the Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism) | + +## Expansion mechanisms + +There are three expansion mechanisms: + +- GitLab +- GitLab Runner +- Execution shell environment + +### GitLab internal variable expansion mechanism + +The expanded part needs to be in a form of `$variable`, or `${variable}` or `%variable%`. +Each form is handled in the same way, no matter which OS/shell will finally handle the job, +since the expansion is done in GitLab before any Runner will get the job. + +### GitLab Runner internal variable expansion mechanism + +- **Supported:** secret variables, `.gitlab-ci.yml` variables, `config.toml` variables, and + variables from triggers and pipeline schedules +- **Not supported:** variables defined inside of scripts (e.g., `export MY_VARIABLE="test"`) + +The Runner uses Go's `os.Expand()` method for variable expansion. It means that it will handle +only variables defined as `$variable` and `${variable}`. What's also important, is that +the expansion is done only once, so nested variables may or may not work, depending on the +ordering of variables definitions. + +### Execution shell environment + +This is an expansion that takes place during the `script` execution. +How it works depends on the used shell (bash/sh/cmd/PowerShell). For example, if the job's +`script` contains a line `echo $MY_VARIABLE-${MY_VARIABLE_2}`, it should be properly handled +by bash/sh (leaving empty strings or some values depending whether the variables were +defined or not), but will not work with Windows' cmd/PowerShell, since these shells +are using a different variables syntax. + +**Supported:** + +- The `script` may use all available variables that are default for the shell (e.g., `$PATH` which + should be present in all bash/sh shells) and all variables defined by GitLab CI/CD (secret variables, + `.gitlab-ci.yml` variables, `config.toml` variables, and variables from triggers and pipeline schedules). +- The `script` may also use all variables defined in the lines before. So, for example, if you define + a variable `export MY_VARIABLE="test"`: + + - in `before_script`, it will work in the following lines of `before_script` and + all lines of the related `script` + - in `script`, it will work in the following lines of `script` + - in `after_script`, it will work in following lines of `after_script` + +## Persisted variables + +NOTE: **Note:** +Some of the persisted variables contain tokens and cannot be used by some definitions +due to security reasons. + +The following variables are known as "persisted": + +- `CI_PIPELINE_ID` +- `CI_JOB_ID` +- `CI_JOB_TOKEN` +- `CI_BUILD_ID` +- `CI_BUILD_TOKEN` +- `CI_REGISTRY_USER` +- `CI_REGISTRY_PASSWORD` +- `CI_REPOSITORY_URL` +- `CI_DEPLOY_USER` +- `CI_DEPLOY_PASSWORD` + +They are: + +- **supported** for all definitions as [described in the table](#gitlab-ci-yml-file) where the "Expansion place" is "Runner" +- **not supported:** + - by the definitions [described in the table](#gitlab-ci-yml-file) where the "Expansion place" is "GitLab" + - in the `only` and `except` [variables expressions](README.md#variables-expressions) diff --git a/doc/development/ux_guide/components.md b/doc/development/ux_guide/components.md index b57520a00e0..4a3b3125f59 100644 --- a/doc/development/ux_guide/components.md +++ b/doc/development/ux_guide/components.md @@ -193,7 +193,7 @@ List with avatar, title and description using .content-list ![List with avatar](img/components-listwithavatar.png) -List with hover effect .well-list +List with hover effect .content-list ![List with hover effect](img/components-listwithhover.png) diff --git a/doc/user/project/merge_requests/img/squash_edit_form.png b/doc/user/project/merge_requests/img/squash_edit_form.png Binary files differnew file mode 100644 index 00000000000..496c6f44ea7 --- /dev/null +++ b/doc/user/project/merge_requests/img/squash_edit_form.png diff --git a/doc/user/project/merge_requests/img/squash_mr_commits.png b/doc/user/project/merge_requests/img/squash_mr_commits.png Binary files differnew file mode 100644 index 00000000000..5fc6a8c48bb --- /dev/null +++ b/doc/user/project/merge_requests/img/squash_mr_commits.png diff --git a/doc/user/project/merge_requests/img/squash_mr_widget.png b/doc/user/project/merge_requests/img/squash_mr_widget.png Binary files differnew file mode 100644 index 00000000000..9cb458b2a35 --- /dev/null +++ b/doc/user/project/merge_requests/img/squash_mr_widget.png diff --git a/doc/user/project/merge_requests/img/squash_squashed_commit.png b/doc/user/project/merge_requests/img/squash_squashed_commit.png Binary files differnew file mode 100644 index 00000000000..0cf5875f82c --- /dev/null +++ b/doc/user/project/merge_requests/img/squash_squashed_commit.png diff --git a/doc/user/project/merge_requests/index.md b/doc/user/project/merge_requests/index.md index 5932f5a2bc1..b75bcacc9d7 100644 --- a/doc/user/project/merge_requests/index.md +++ b/doc/user/project/merge_requests/index.md @@ -29,12 +29,12 @@ With GitLab merge requests, you can: - Enable [semi-linear history merge requests](#semi-linear-history-merge-requests) as another security layer to guarantee the pipeline is passing in the target branch - [Create new merge requests by email](#create-new-merge-requests-by-email) - Allow maintainers of the target project to push directly to the fork by [allowing edits from maintainers](maintainer_access.md) +- [Squash and merge](squash_and_merge.md) for a cleaner commit history With **[GitLab Enterprise Edition][ee]**, you can also: - View the deployment process across projects with [Multi-Project Pipeline Graphs](https://docs.gitlab.com/ee/ci/multi_project_pipeline_graphs.html#multi-project-pipeline-graphs) **[PREMIUM]** - Request [approvals](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html) from your managers **[STARTER]** -- [Squash and merge](https://docs.gitlab.com/ee/user/project/merge_requests/squash_and_merge.html) for a cleaner commit history **[STARTER]** - Analyze the impact of your changes with [Code Quality reports](https://docs.gitlab.com/ee/user/project/merge_requests/code_quality_diff.html) **[STARTER]** ## Use cases @@ -57,7 +57,7 @@ B. Consider you're a web developer writing a webpage for your company's: 1. Your changes are previewed with [Review Apps](../../../ci/review_apps/index.md) 1. You request your web designers for their implementation 1. You request the [approval](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html) from your manager **[STARTER]** -1. Once approved, your merge request is [squashed and merged](https://docs.gitlab.com/ee/user/project/merge_requests/squash_and_merge.html), and [deployed to staging with GitLab Pages](https://about.gitlab.com/2016/08/26/ci-deployment-and-environments/) (Squash and Merge is available in GitLab Starter) +1. Once approved, your merge request is [squashed and merged](squash_and_merge.md), and [deployed to staging with GitLab Pages](https://about.gitlab.com/2016/08/26/ci-deployment-and-environments/) 1. Your production team [cherry picks](#cherry-pick-changes) the merge commit into production ## Merge requests per project diff --git a/doc/user/project/merge_requests/squash_and_merge.md b/doc/user/project/merge_requests/squash_and_merge.md new file mode 100644 index 00000000000..a6efe893853 --- /dev/null +++ b/doc/user/project/merge_requests/squash_and_merge.md @@ -0,0 +1,80 @@ +# Squash and merge + +> [Introduced][ee-1024] in [GitLab Starter][ee] 8.17, and in [GitLab CE][ce] [11.0][ce-18956]. + +Combine all commits of your merge request into one and retain a clean history. + +## Overview + +Squashing lets you tidy up the commit history of a branch when accepting a merge +request. It applies all of the changes in the merge request as a single commit, +and then merges that commit using the merge method set for the project. + +In other words, squashing a merge request turns a long list of commits: + +![List of commits from a merge request][mr-commits] + +Into a single commit on merge: + +![A squashed commit followed by a merge commit][squashed-commit] + +The squashed commit's commit message is the merge request title. And note that +the squashed commit is still followed by a merge commit, as the merge +method for this example repository uses a merge commit. Squashing also works +with the fast-forward merge strategy, see +[squashing and fast-forward merge](#squash-and-fast-forward-merge) for more +details. + +## Use cases + +When working on a feature branch, you sometimes want to commit your current +progress, but don't really care about the commit messages. Those 'work in +progress commits' don't necessarily contain important information and as such +you'd rather not include them in your target branch. + +With squash and merge, when the merge request is ready to be merged, +all you have to do is enable squashing before you press merge to join +the commits include in the merge request into a single commit. + +This way, the history of your base branch remains clean with +meaningful commit messages and is simpler to [revert] if necessary. + +## Enabling squash for a merge request + +Anyone who can create or edit a merge request can choose for it to be squashed +on the merge request form: + +![Squash commits checkbox on edit form][squash-edit-form] + +--- + +This can then be overridden at the time of accepting the merge request: + +![Squash commits checkbox on accept merge request form][squash-mr-widget] + +## Commit metadata for squashed commits + +The squashed commit has the following metadata: + +* Message: the title of the merge request. +* Author: the author of the merge request. +* Committer: the user who initiated the squash. + +## Squash and fast-forward merge + +When a project has the [fast-forward merge setting enabled][ff-merge], the merge +request must be able to be fast-forwarded without squashing in order to squash +it. This is because squashing is only available when accepting a merge request, +so a merge request may need to be rebased before squashing, even though +squashing can itself be considered equivalent to rebasing. + +[ee-1024]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1024 +[ce-18956]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/18956 +[mr-commits]: img/squash_mr_commits.png +[squashed-commit]: img/squash_squashed_commit.png +[squash-edit-form]: img/squash_edit_form.png +[squash-mr-widget]: img/squash_mr_widget.png +[ff-merge]: fast_forward_merge.md#enabling-fast-forward-merges +[ce]: https://about.gitlab.com/products/ +[ee]: https://about.gitlab.com/products/ +[revert]: revert_changes.md diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index 70d43ac1d79..b7aadc27e71 100644 --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -148,10 +148,10 @@ module API requires :key_id, type: Integer, desc: 'The ID of the deploy key' end delete ":id/deploy_keys/:key_id" do - key = user_project.deploy_keys.find(params[:key_id]) - not_found!('Deploy Key') unless key + deploy_key_project = user_project.deploy_keys_projects.find_by(deploy_key_id: params[:key_id]) + not_found!('Deploy Key') unless deploy_key_project - destroy_conditionally!(key) + destroy_conditionally!(deploy_key_project) end end end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 3e615f7ac05..49cd4fccc63 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -568,6 +568,8 @@ module API expose :time_stats, using: 'API::Entities::IssuableTimeStats' do |merge_request| merge_request end + + expose :squash end class MergeRequest < MergeRequestBasic @@ -933,8 +935,16 @@ module API end class ApplicationSetting < Grape::Entity - expose :id - expose(*::ApplicationSettingsHelper.visible_attributes) + def self.exposed_attributes + attributes = ::ApplicationSettingsHelper.visible_attributes + attributes.delete(:performance_bar_allowed_group_path) + attributes.delete(:performance_bar_enabled) + + attributes + end + + expose :id, :performance_bar_allowed_group_id + expose(*exposed_attributes) expose(:restricted_visibility_levels) do |setting, _options| setting.restricted_visibility_levels.map { |level| Gitlab::VisibilityLevel.string_level(level) } end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index bc4df16e3a8..1ba9a09346f 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -10,12 +10,6 @@ module API helpers do params :optional_params_ee do end - - params :merge_params_ee do - end - - def update_merge_request_ee(merge_request) - end end def self.update_params_at_least_one_of @@ -29,6 +23,7 @@ module API target_branch title discussion_locked + squash ] end @@ -146,6 +141,7 @@ module API optional :labels, type: String, desc: 'Comma-separated list of label names' optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging' optional :allow_maintainer_to_push, type: Boolean, desc: 'Whether a maintainer of the target project can push to the source project' + optional :squash, type: Grape::API::Boolean, desc: 'When true, the commits will be squashed into a single commit on merge' use :optional_params_ee end @@ -308,8 +304,7 @@ module API optional :merge_when_pipeline_succeeds, type: Boolean, desc: 'When true, this merge request will be merged when the pipeline succeeds' optional :sha, type: String, desc: 'When present, must have the HEAD SHA of the source branch' - - use :merge_params_ee + optional :squash, type: Grape::API::Boolean, desc: 'When true, the commits will be squashed into a single commit on merge' end put ':id/merge_requests/:merge_request_iid/merge' do Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42317') @@ -327,7 +322,7 @@ module API check_sha_param!(params, merge_request) - update_merge_request_ee(merge_request) + merge_request.update(squash: params[:squash]) if params[:squash] merge_params = { commit_message: params[:merge_commit_message], diff --git a/lib/api/settings.rb b/lib/api/settings.rb index d727ad59367..4b506127c0e 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -49,6 +49,9 @@ module API optional :signin_enabled, type: Boolean, desc: 'Flag indicating if password authentication is enabled for the web interface' # support legacy names, can be removed in v5 mutually_exclusive :password_authentication_enabled_for_web, :password_authentication_enabled, :signin_enabled optional :password_authentication_enabled_for_git, type: Boolean, desc: 'Flag indicating if password authentication is enabled for Git over HTTP(S)' + optional :performance_bar_allowed_group_path, type: String, desc: 'Path of the group that is allowed to toggle the performance bar.' + optional :performance_bar_allowed_group_id, type: String, desc: 'Depreated: Use :performance_bar_allowed_group_path instead. Path of the group that is allowed to toggle the performance bar.' # support legacy names, can be removed in v6 + optional :performance_bar_enabled, type: String, desc: 'Deprecated: Pass `performance_bar_allowed_group_path: nil` instead. Allow enabling the performance.' # support legacy names, can be removed in v6 optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to setup Two-factor authentication' given require_two_factor_authentication: ->(val) { val } do requires :two_factor_grace_period, type: Integer, desc: 'Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication' @@ -134,12 +137,25 @@ module API desc: "Restrictions on the complexity of uploaded #{type.upcase} keys. A value of #{ApplicationSetting::FORBIDDEN_KEY_VALUE} disables all #{type.upcase} keys." end - optional(*::ApplicationSettingsHelper.visible_attributes) - at_least_one_of(*::ApplicationSettingsHelper.visible_attributes) + optional_attributes = ::ApplicationSettingsHelper.visible_attributes << :performance_bar_allowed_group_id + + optional(*optional_attributes) + at_least_one_of(*optional_attributes) end put "application/settings" do attrs = declared_params(include_missing: false) + # support legacy names, can be removed in v6 + if attrs.has_key?(:performance_bar_allowed_group_id) + attrs[:performance_bar_allowed_group_path] = attrs.delete(:performance_bar_allowed_group_id) + end + + # support legacy names, can be removed in v6 + if attrs.has_key?(:performance_bar_enabled) + performance_bar_enabled = attrs.delete(:performance_bar_allowed_group_id) + attrs[:performance_bar_allowed_group_path] = nil unless performance_bar_enabled + end + # support legacy names, can be removed in v5 if attrs.has_key?(:signin_enabled) attrs[:password_authentication_enabled_for_web] = attrs.delete(:signin_enabled) diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index 68b4d7c3982..28fcf6c6e84 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -134,6 +134,8 @@ module API expose :should_remove_source_branch?, as: :should_remove_source_branch expose :force_remove_source_branch?, as: :force_remove_source_branch + expose :squash + expose :web_url do |merge_request, options| Gitlab::UrlBuilder.build(merge_request) end diff --git a/lib/api/v3/merge_requests.rb b/lib/api/v3/merge_requests.rb index 9b0f70e2bfe..af5afd1c334 100644 --- a/lib/api/v3/merge_requests.rb +++ b/lib/api/v3/merge_requests.rb @@ -44,6 +44,7 @@ module API optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign the merge request' optional :labels, type: String, desc: 'Comma-separated list of label names' optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging' + optional :squash, type: Boolean, desc: 'Squash commits when merging' end end @@ -166,7 +167,7 @@ module API use :optional_params at_least_one_of :title, :target_branch, :description, :assignee_id, :milestone_id, :labels, :state_event, - :remove_source_branch + :remove_source_branch, :squash end put path do Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42127') @@ -195,6 +196,7 @@ module API optional :merge_when_build_succeeds, type: Boolean, desc: 'When true, this merge request will be merged when the build succeeds' optional :sha, type: String, desc: 'When present, must have the HEAD SHA of the source branch' + optional :squash, type: Boolean, desc: 'When true, the commits will be squashed into a single commit on merge' end put "#{path}/merge" do merge_request = find_project_merge_request(params[:merge_request_id]) @@ -211,6 +213,8 @@ module API render_api_error!("SHA does not match HEAD of source branch: #{merge_request.diff_head_sha}", 409) end + merge_request.update(squash: params[:squash]) if params[:squash] + merge_params = { commit_message: params[:merge_commit_message], should_remove_source_branch: params[:should_remove_source_branch] diff --git a/lib/gitlab/ee_compat_check.rb b/lib/gitlab/ee_compat_check.rb index 8cf59fa8e28..8c72d00c1f3 100644 --- a/lib/gitlab/ee_compat_check.rb +++ b/lib/gitlab/ee_compat_check.rb @@ -138,8 +138,8 @@ module Gitlab def ee_branch_presence_check! ee_remotes.keys.each do |remote| - [ee_branch_prefix, ee_branch_suffix].each do |branch| - _, status = step("Fetching #{remote}/#{ee_branch_prefix}", %W[git fetch #{remote} #{branch}]) + [ce_branch, ee_branch_prefix, ee_branch_suffix].each do |branch| + _, status = step("Fetching #{remote}/#{branch}", %W[git fetch #{remote} #{branch}]) if status.zero? @ee_remote_with_branch = remote diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb index 34169319b26..7c9fc5c15bb 100644 --- a/lib/gitlab/import_export/attribute_cleaner.rb +++ b/lib/gitlab/import_export/attribute_cleaner.rb @@ -7,14 +7,15 @@ module Gitlab new(*args).clean end - def initialize(relation_hash:, relation_class:) + def initialize(relation_hash:, relation_class:, excluded_keys: []) @relation_hash = relation_hash @relation_class = relation_class + @excluded_keys = excluded_keys end def clean @relation_hash.reject do |key, _value| - prohibited_key?(key) || !@relation_class.attribute_method?(key) + prohibited_key?(key) || !@relation_class.attribute_method?(key) || excluded_key?(key) end.except('id') end @@ -23,6 +24,12 @@ module Gitlab def prohibited_key?(key) key.end_with?('_id') && !ALLOWED_REFERENCES.include?(key) end + + def excluded_key?(key) + return false if @excluded_keys.empty? + + @excluded_keys.include?(key) + end end end end diff --git a/lib/gitlab/import_export/attributes_finder.rb b/lib/gitlab/import_export/attributes_finder.rb index 56042ddecbf..0c8fda07294 100644 --- a/lib/gitlab/import_export/attributes_finder.rb +++ b/lib/gitlab/import_export/attributes_finder.rb @@ -32,6 +32,10 @@ module Gitlab @methods[key].nil? ? {} : { methods: @methods[key] } end + def find_excluded_keys(klass_name) + @excluded_attributes[klass_name.to_sym]&.map(&:to_s) || [] + end + private def find_attributes_only(value) diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 21ac7f7e0b6..36c7534cd7a 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -98,8 +98,6 @@ excluded_attributes: - :import_jid - :created_at - :updated_at - - :import_jid - - :import_jid - :id - :star_count - :last_activity_at diff --git a/lib/gitlab/import_export/project_tree_restorer.rb b/lib/gitlab/import_export/project_tree_restorer.rb index d5590dde40f..4eb67fbe11e 100644 --- a/lib/gitlab/import_export/project_tree_restorer.rb +++ b/lib/gitlab/import_export/project_tree_restorer.rb @@ -88,16 +88,18 @@ module Gitlab end def project_params - @project_params ||= json_params.merge(override_params) + @project_params ||= begin + attrs = json_params.merge(override_params) + + # Cleaning all imported and overridden params + Gitlab::ImportExport::AttributeCleaner.clean(relation_hash: attrs, + relation_class: Project, + excluded_keys: excluded_keys_for_relation(:project)) + end end def override_params - return {} unless params = @project.import_data&.data&.fetch('override_params', nil) - - @override_params ||= params.select do |key, _value| - Project.column_names.include?(key.to_s) && - !reader.project_tree[:except].include?(key.to_sym) - end + @override_params ||= @project.import_data&.data&.fetch('override_params', nil) || {} end def json_params @@ -171,7 +173,8 @@ module Gitlab relation_hash: parsed_relation_hash(relation_hash, relation.to_sym), members_mapper: members_mapper, user: @user, - project: @restored_project) + project: @restored_project, + excluded_keys: excluded_keys_for_relation(relation)) end.compact relation_hash_list.is_a?(Array) ? relation_array : relation_array.first @@ -192,6 +195,10 @@ module Gitlab def reader @reader ||= Gitlab::ImportExport::Reader.new(shared: @shared) end + + def excluded_keys_for_relation(relation) + @reader.attributes_finder.find_excluded_keys(relation) + end end end end diff --git a/lib/gitlab/import_export/reader.rb b/lib/gitlab/import_export/reader.rb index eb7f5120592..e621c40fc7a 100644 --- a/lib/gitlab/import_export/reader.rb +++ b/lib/gitlab/import_export/reader.rb @@ -1,7 +1,7 @@ module Gitlab module ImportExport class Reader - attr_reader :tree + attr_reader :tree, :attributes_finder def initialize(shared:) @shared = shared diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 4a41a69840b..b736b2c3fe5 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -36,13 +36,21 @@ module Gitlab new(*args).create end - def initialize(relation_sym:, relation_hash:, members_mapper:, user:, project:) + def initialize(relation_sym:, relation_hash:, members_mapper:, user:, project:, excluded_keys: []) @relation_name = OVERRIDES[relation_sym] || relation_sym @relation_hash = relation_hash.except('noteable_id') @members_mapper = members_mapper @user = user @project = project @imported_object_retries = 0 + + # Remove excluded keys from relation_hash + # We don't do this in the parsed_relation_hash because of the 'transformed attributes' + # For example, MergeRequestDiffFiles exports its diff attribute as utf8_diff. Then, + # in the create method that attribute is renamed to diff. And because diff is an excluded key, + # if we clean the excluded keys in the parsed_relation_hash, it will be removed + # from the object attributes and the export will fail. + @relation_hash.except!(*excluded_keys) end # Creates an object from an actual model with name "relation_sym" with params from diff --git a/qa/qa/page/merge_request/show.rb b/qa/qa/page/merge_request/show.rb index 166861e6c4a..9507f92f4b2 100644 --- a/qa/qa/page/merge_request/show.rb +++ b/qa/qa/page/merge_request/show.rb @@ -16,6 +16,10 @@ module QA element :no_fast_forward_message, 'Fast-forward merge is not possible' end + view 'app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_squash_before_merge.vue' do + element :squash_checkbox + end + def rebase! click_element :mr_rebase_button @@ -41,6 +45,14 @@ module QA has_text?('The changes were merged into') end end + + def mark_to_squash + wait(reload: true) do + has_css?(element_selector_css(:squash_checkbox)) + end + + click_element :squash_checkbox + end end end end diff --git a/qa/qa/specs/features/merge_request/squash_spec.rb b/qa/qa/specs/features/merge_request/squash_spec.rb new file mode 100644 index 00000000000..dbbdf852a38 --- /dev/null +++ b/qa/qa/specs/features/merge_request/squash_spec.rb @@ -0,0 +1,48 @@ +module QA + feature 'merge request squash commits', :core do + scenario 'when squash commits is marked before merge' do + Runtime::Browser.visit(:gitlab, Page::Main::Login) + Page::Main::Login.act { sign_in_using_credentials } + + project = Factory::Resource::Project.fabricate! do |project| + project.name = "squash-before-merge" + end + + merge_request = Factory::Resource::MergeRequest.fabricate! do |merge_request| + merge_request.project = project + merge_request.title = 'Squashing commits' + end + + Factory::Repository::Push.fabricate! do |push| + push.project = project + push.commit_message = 'to be squashed' + push.branch_name = merge_request.source_branch + push.new_branch = false + push.file_name = 'other.txt' + push.file_content = "Test with unicode characters ❤✓€❄" + end + + merge_request.visit! + + Page::MergeRequest::Show.perform do |merge_request_page| + merge_request_page.mark_to_squash + merge_request_page.merge! + + merge_request.project.visit! + + Git::Repository.perform do |repository| + repository.uri = Page::Project::Show.act do + choose_repository_clone_http + repository_location.uri + end + + repository.use_default_credentials + + repository.act { clone } + + expect(repository.commits.size).to eq 3 + end + end + end + end +end diff --git a/rubocop/cop/line_break_around_conditional_block.rb b/rubocop/cop/line_break_around_conditional_block.rb index 3e7021e724e..8b6052fee1b 100644 --- a/rubocop/cop/line_break_around_conditional_block.rb +++ b/rubocop/cop/line_break_around_conditional_block.rb @@ -95,7 +95,7 @@ module RuboCop end def end_clause_line?(line) - line =~ /^\s*(rescue|else|elsif|when)/ + line =~ /^\s*(#|rescue|else|elsif|when)/ end def begin_line?(line) diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb index 4770e187db6..dcb0faffbd4 100644 --- a/spec/controllers/boards/issues_controller_spec.rb +++ b/spec/controllers/boards/issues_controller_spec.rb @@ -17,7 +17,7 @@ describe Boards::IssuesController do project.add_guest(guest) end - describe 'GET index' do + describe 'GET index', :request_store do let(:johndoe) { create(:user, avatar: fixture_file_upload(File.join(Rails.root, 'spec/fixtures/dk.png'))) } context 'with invalid board id' do diff --git a/spec/controllers/profiles_controller_spec.rb b/spec/controllers/profiles_controller_spec.rb index c621eb69171..4530a301d4d 100644 --- a/spec/controllers/profiles_controller_spec.rb +++ b/spec/controllers/profiles_controller_spec.rb @@ -3,6 +3,19 @@ require('spec_helper') describe ProfilesController, :request_store do let(:user) { create(:user) } + describe 'POST update' do + it 'does not update password' do + sign_in(user) + + expect do + post :update, + user: { password: 'hello12345', password_confirmation: 'hello12345' } + end.not_to change { user.reload.encrypted_password } + + expect(response.status).to eq(302) + end + end + describe 'PUT update' do it 'allows an email update from a user without an external email address' do sign_in(user) diff --git a/spec/controllers/projects/boards_controller_spec.rb b/spec/controllers/projects/boards_controller_spec.rb index 4d765229bde..509f19ed030 100644 --- a/spec/controllers/projects/boards_controller_spec.rb +++ b/spec/controllers/projects/boards_controller_spec.rb @@ -27,6 +27,20 @@ describe Projects::BoardsController do expect(response).to render_template :index expect(response.content_type).to eq 'text/html' end + + context 'with unauthorized user' do + before do + allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) + allow(Ability).to receive(:allowed?).with(user, :read_board, project).and_return(false) + end + + it 'returns a not found 404 response' do + list_boards + + expect(response).to have_gitlab_http_status(404) + expect(response.content_type).to eq 'text/html' + end + end end context 'when format is JSON' do @@ -40,18 +54,19 @@ describe Projects::BoardsController do expect(response).to match_response_schema('boards') expect(parsed_response.length).to eq 2 end - end - context 'with unauthorized user' do - before do - allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) - allow(Ability).to receive(:allowed?).with(user, :read_board, project).and_return(false) - end + context 'with unauthorized user' do + before do + allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) + allow(Ability).to receive(:allowed?).with(user, :read_board, project).and_return(false) + end - it 'returns a not found 404 response' do - list_boards + it 'returns a not found 404 response' do + list_boards format: :json - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(404) + expect(response.content_type).to eq 'application/json' + end end end @@ -88,6 +103,20 @@ describe Projects::BoardsController do expect(response).to render_template :show expect(response.content_type).to eq 'text/html' end + + context 'with unauthorized user' do + before do + allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) + allow(Ability).to receive(:allowed?).with(user, :read_board, project).and_return(false) + end + + it 'returns a not found 404 response' do + read_board board: board + + expect(response).to have_gitlab_http_status(404) + expect(response.content_type).to eq 'text/html' + end + end end context 'when format is JSON' do @@ -96,18 +125,19 @@ describe Projects::BoardsController do expect(response).to match_response_schema('board') end - end - context 'with unauthorized user' do - before do - allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) - allow(Ability).to receive(:allowed?).with(user, :read_board, project).and_return(false) - end + context 'with unauthorized user' do + before do + allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) + allow(Ability).to receive(:allowed?).with(user, :read_board, project).and_return(false) + end - it 'returns a not found 404 response' do - read_board board: board + it 'returns a not found 404 response' do + read_board board: board, format: :json - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(404) + expect(response.content_type).to eq 'application/json' + end end end diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index 16fb377b002..4860ea5dcce 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -146,6 +146,24 @@ describe Projects::BranchesController do it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes' end + + it 'redirects to autodeploy setup page' do + result = { status: :success, branch: double(name: branch) } + + create(:cluster, :provided_by_gcp, projects: [project]) + + expect_any_instance_of(CreateBranchService).to receive(:execute).and_return(result) + expect(SystemNoteService).to receive(:new_issue_branch).and_return(true) + + post :create, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + branch_name: branch, + issue_iid: issue.iid + + expect(response.location).to include(project_new_blob_path(project, branch)) + expect(response).to have_gitlab_http_status(302) + end end context 'when create branch service fails' do diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index 82b20e12850..380e50c8cac 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -2,7 +2,6 @@ require 'spec_helper' describe Projects::ClustersController do include AccessMatchersForController - include GoogleApi::CloudPlatformHelpers set(:project) { create(:project) } @@ -333,7 +332,7 @@ describe Projects::ClustersController do context 'when cluster is provided by GCP' do context 'when cluster is created' do - let!(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } + let!(:cluster) { create(:cluster, :provided_by_gcp, :production_environment, projects: [project]) } it "destroys and redirects back to clusters list" do expect { go } @@ -347,7 +346,7 @@ describe Projects::ClustersController do end context 'when cluster is being created' do - let!(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) } + let!(:cluster) { create(:cluster, :providing_by_gcp, :production_environment, projects: [project]) } it "destroys and redirects back to clusters list" do expect { go } @@ -361,7 +360,7 @@ describe Projects::ClustersController do end context 'when cluster is provided by user' do - let!(:cluster) { create(:cluster, :provided_by_user, projects: [project]) } + let!(:cluster) { create(:cluster, :provided_by_user, :production_environment, projects: [project]) } it "destroys and redirects back to clusters list" do expect { go } @@ -376,7 +375,7 @@ describe Projects::ClustersController do end describe 'security' do - set(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } + set(:cluster) { create(:cluster, :provided_by_gcp, :production_environment, projects: [project]) } it { expect { go }.to be_allowed_for(:admin) } it { expect { go }.to be_allowed_for(:owner).of(project) } diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index ff9ab53d8c3..47d4942acbd 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -21,6 +21,13 @@ describe Projects::EnvironmentsController do expect(response).to have_gitlab_http_status(:ok) end + + it 'expires etag cache to force reload environments list' do + expect_any_instance_of(Gitlab::EtagCaching::Store) + .to receive(:touch).with(project_environments_path(project, format: :json)) + + get :index, environment_params + end end context 'when requesting JSON response for folders' do diff --git a/spec/controllers/projects/group_links_controller_spec.rb b/spec/controllers/projects/group_links_controller_spec.rb index 5bfc3d31401..72f6af112b3 100644 --- a/spec/controllers/projects/group_links_controller_spec.rb +++ b/spec/controllers/projects/group_links_controller_spec.rb @@ -21,6 +21,18 @@ describe Projects::GroupLinksController do end end + context 'when project is not allowed to be shared with a group' do + before do + group.update_attributes(share_with_group_lock: false) + end + + include_context 'link project to group' + + it 'responds with status 404' do + expect(response).to have_gitlab_http_status(404) + end + end + context 'when user has access to group he want to link project to' do before do group.add_developer(user) diff --git a/spec/controllers/projects/imports_controller_spec.rb b/spec/controllers/projects/imports_controller_spec.rb index 7fb4c1b7425..011843baffc 100644 --- a/spec/controllers/projects/imports_controller_spec.rb +++ b/spec/controllers/projects/imports_controller_spec.rb @@ -2,16 +2,15 @@ require 'spec_helper' describe Projects::ImportsController do let(:user) { create(:user) } + let(:project) { create(:project) } + + before do + sign_in(user) + project.add_master(user) + end describe 'GET #show' do context 'when repository does not exists' do - let(:project) { create(:project) } - - before do - sign_in(user) - project.add_master(user) - end - it 'renders template' do get :show, namespace_id: project.namespace.to_param, project_id: project @@ -28,11 +27,6 @@ describe Projects::ImportsController do context 'when repository exists' do let(:project) { create(:project_empty_repo, import_url: 'https://github.com/vim/vim.git') } - before do - sign_in(user) - project.add_master(user) - end - context 'when import is in progress' do before do project.update_attribute(:import_status, :started) diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index ca86b0bc737..106611b37c9 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1,4 +1,4 @@ -require('spec_helper') +require 'spec_helper' describe Projects::IssuesController do let(:project) { create(:project) } diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index d3042be9e8b..5efaaf2bb3a 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -275,6 +275,7 @@ describe Projects::MergeRequestsController do namespace_id: project.namespace, project_id: project, id: merge_request.iid, + squash: false, format: 'json' } end @@ -315,8 +316,8 @@ describe Projects::MergeRequestsController do end context 'when the sha parameter matches the source SHA' do - def merge_with_sha - post :merge, base_params.merge(sha: merge_request.diff_head_sha) + def merge_with_sha(params = {}) + post :merge, base_params.merge(sha: merge_request.diff_head_sha).merge(params) end it 'returns :success' do @@ -331,6 +332,24 @@ describe Projects::MergeRequestsController do merge_with_sha end + context 'when squash is passed as 1' do + it 'updates the squash attribute on the MR to true' do + merge_request.update(squash: false) + merge_with_sha(squash: '1') + + expect(merge_request.reload.squash).to be_truthy + end + end + + context 'when squash is passed as 0' do + it 'updates the squash attribute on the MR to false' do + merge_request.update(squash: true) + merge_with_sha(squash: '0') + + expect(merge_request.reload.squash).to be_falsey + end + end + context 'when the pipeline succeeds is passed' do let!(:head_pipeline) do create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch, head_pipeline_of: merge_request) diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 46b08a03b19..d84b31ad978 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -184,7 +184,7 @@ describe Projects::ProjectMembersController do project.add_master(user) end - it 'cannot remove himself from the project' do + it 'cannot remove themselves from the project' do delete :leave, namespace_id: project.namespace, project_id: project diff --git a/spec/factories/application_settings.rb b/spec/factories/application_settings.rb index 3ecc90b6573..00c063c49f8 100644 --- a/spec/factories/application_settings.rb +++ b/spec/factories/application_settings.rb @@ -1,4 +1,5 @@ FactoryBot.define do factory :application_setting do + default_projects_limit 42 end end diff --git a/spec/features/merge_requests/user_squashes_merge_request_spec.rb b/spec/features/merge_requests/user_squashes_merge_request_spec.rb new file mode 100644 index 00000000000..6c952791591 --- /dev/null +++ b/spec/features/merge_requests/user_squashes_merge_request_spec.rb @@ -0,0 +1,124 @@ +require 'spec_helper' + +feature 'User squashes a merge request', :js do + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + let(:source_branch) { 'csv' } + + let!(:original_head) { project.repository.commit('master') } + + shared_examples 'squash' do + it 'squashes the commits into a single commit, and adds a merge commit' do + expect(page).to have_content('Merged') + + latest_master_commits = project.repository.commits_between(original_head.sha, 'master').map(&:raw) + + squash_commit = an_object_having_attributes(sha: a_string_matching(/\h{40}/), + message: "Csv\n", + author_name: user.name, + committer_name: user.name) + + merge_commit = an_object_having_attributes(sha: a_string_matching(/\h{40}/), + message: a_string_starting_with("Merge branch 'csv' into 'master'"), + author_name: user.name, + committer_name: user.name) + + expect(project.repository).not_to be_merged_to_root_ref(source_branch) + expect(latest_master_commits).to match([squash_commit, merge_commit]) + end + end + + shared_examples 'no squash' do + it 'accepts the merge request without squashing' do + expect(page).to have_content('Merged') + expect(project.repository).to be_merged_to_root_ref(source_branch) + end + end + + def accept_mr + expect(page).to have_button('Merge') + + uncheck 'Remove source branch' + click_on 'Merge' + end + + before do + # Prevent source branch from being removed so we can use be_merged_to_root_ref + # method to check if squash was performed or not + allow_any_instance_of(MergeRequest).to receive(:force_remove_source_branch?).and_return(false) + project.add_master(user) + + sign_in user + end + + context 'when the MR has only one commit' do + before do + merge_request = create(:merge_request, source_project: project, target_project: project, source_branch: 'master', target_branch: 'branch-merged') + + visit project_merge_request_path(project, merge_request) + end + + it 'does not show the squash checkbox' do + expect(page).not_to have_field('squash') + end + end + + context 'when squash is enabled on merge request creation' do + before do + visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: source_branch }) + check 'merge_request[squash]' + click_on 'Submit merge request' + wait_for_requests + end + + it 'shows the squash checkbox as checked' do + expect(page).to have_checked_field('squash') + end + + context 'when accepting with squash checked' do + before do + accept_mr + end + + include_examples 'squash' + end + + context 'when accepting and unchecking squash' do + before do + uncheck 'squash' + accept_mr + end + + include_examples 'no squash' + end + end + + context 'when squash is not enabled on merge request creation' do + before do + visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: source_branch }) + click_on 'Submit merge request' + wait_for_requests + end + + it 'shows the squash checkbox as unchecked' do + expect(page).to have_unchecked_field('squash') + end + + context 'when accepting and checking squash' do + before do + check 'squash' + accept_mr + end + + include_examples 'squash' + end + + context 'when accepting with squash unchecked' do + before do + accept_mr + end + + include_examples 'no squash' + end + end +end diff --git a/spec/features/projects/merge_requests/user_views_diffs_spec.rb b/spec/features/projects/merge_requests/user_views_diffs_spec.rb index 295eb02b625..d36aafdbc54 100644 --- a/spec/features/projects/merge_requests/user_views_diffs_spec.rb +++ b/spec/features/projects/merge_requests/user_views_diffs_spec.rb @@ -26,6 +26,10 @@ describe 'User views diffs', :js do expect(page).to have_css('#inline-diff-btn', count: 1) end + it 'hides loading spinner after load' do + expect(page).not_to have_selector('.mr-loading-status .loading', visible: true) + end + context 'when in the inline view' do include_examples 'unfold diffs' end diff --git a/spec/features/users/user_browses_projects_on_user_page_spec.rb b/spec/features/users/user_browses_projects_on_user_page_spec.rb index 7bede0b0d48..5478e38ce70 100644 --- a/spec/features/users/user_browses_projects_on_user_page_spec.rb +++ b/spec/features/users/user_browses_projects_on_user_page_spec.rb @@ -26,18 +26,23 @@ describe 'Users > User browses projects on user page', :js do end end + it 'hides loading spinner after load', :js do + visit user_path(user) + click_nav_link('Personal projects') + + wait_for_requests + + expect(page).not_to have_selector('.loading-status .loading', visible: true) + end + it 'paginates projects', :js do project = create(:project, namespace: user.namespace, updated_at: 2.minutes.since) project2 = create(:project, namespace: user.namespace, updated_at: 1.minute.since) allow(Project).to receive(:default_per_page).and_return(1) sign_in(user) - visit user_path(user) - - page.within('.user-profile-nav') do - click_link('Personal projects') - end + click_nav_link('Personal projects') wait_for_requests @@ -92,7 +97,6 @@ describe 'Users > User browses projects on user page', :js do click_nav_link('Personal projects') expect(title).to start_with(user.name) - expect(page).to have_content(private_project.name) expect(page).to have_content(public_project.name) expect(page).to have_content(internal_project.name) diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index 233102c4314..7be8c9e3e67 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -112,7 +112,8 @@ "rebase_commit_sha": { "type": ["string", "null"] }, "rebase_in_progress": { "type": "boolean" }, "can_push_to_source_branch": { "type": "boolean" }, - "rebase_path": { "type": ["string", "null"] } + "rebase_path": { "type": ["string", "null"] }, + "squash": { "type": "boolean" } }, "additionalProperties": false } diff --git a/spec/fixtures/api/schemas/public_api/v3/merge_requests.json b/spec/fixtures/api/schemas/public_api/v3/merge_requests.json index b5c74bcc26e..9af7a8c9f4f 100644 --- a/spec/fixtures/api/schemas/public_api/v3/merge_requests.json +++ b/spec/fixtures/api/schemas/public_api/v3/merge_requests.json @@ -73,7 +73,8 @@ "should_remove_source_branch": { "type": ["boolean", "null"] }, "force_remove_source_branch": { "type": ["boolean", "null"] }, "web_url": { "type": "uri" }, - "subscribed": { "type": ["boolean"] } + "subscribed": { "type": ["boolean"] }, + "squash": { "type": "boolean" } }, "required": [ "id", "iid", "project_id", "title", "description", @@ -83,7 +84,7 @@ "labels", "work_in_progress", "milestone", "merge_when_build_succeeds", "merge_status", "sha", "merge_commit_sha", "user_notes_count", "should_remove_source_branch", "force_remove_source_branch", - "web_url", "subscribed" + "web_url", "subscribed", "squash" ], "additionalProperties": false } diff --git a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json index 0dc2eabec5d..f97461ce9cc 100644 --- a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json +++ b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json @@ -75,6 +75,7 @@ "force_remove_source_branch": { "type": ["boolean", "null"] }, "discussion_locked": { "type": ["boolean", "null"] }, "web_url": { "type": "uri" }, + "squash": { "type": "boolean" }, "time_stats": { "time_estimate": { "type": "integer" }, "total_time_spent": { "type": "integer" }, @@ -91,7 +92,7 @@ "labels", "work_in_progress", "milestone", "merge_when_pipeline_succeeds", "merge_status", "sha", "merge_commit_sha", "user_notes_count", "should_remove_source_branch", "force_remove_source_branch", - "web_url" + "web_url", "squash" ], "additionalProperties": false } diff --git a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb index cd5a1b2982b..536cc359d39 100644 --- a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb +++ b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb @@ -15,7 +15,10 @@ describe Gitlab::ImportExport::AttributeCleaner do 'project_id' => 99, 'user_id' => 99, 'random_id_in_the_middle' => 99, - 'notid' => 99 + 'notid' => 99, + 'import_source' => 'whatever', + 'import_type' => 'whatever', + 'non_existent_attr' => 'whatever' } end @@ -28,10 +31,30 @@ describe Gitlab::ImportExport::AttributeCleaner do } end + let(:excluded_keys) { %w[import_source import_type] } + + subject { described_class.clean(relation_hash: unsafe_hash, relation_class: relation_class, excluded_keys: excluded_keys) } + + before do + allow(relation_class).to receive(:attribute_method?).and_return(true) + allow(relation_class).to receive(:attribute_method?).with('non_existent_attr').and_return(false) + end + it 'removes unwanted attributes from the hash' do - # allow(relation_class).to receive(:attribute_method?).and_return(true) + expect(subject).to eq(post_safe_hash) + end + + it 'removes attributes not present in relation_class' do + expect(subject.keys).not_to include 'non_existent_attr' + end + + it 'removes excluded keys from the hash' do + expect(subject.keys).not_to include excluded_keys + end + + it 'does not remove excluded key if not listed' do parsed_hash = described_class.clean(relation_hash: unsafe_hash, relation_class: relation_class) - expect(parsed_hash).to eq(post_safe_hash) + expect(parsed_hash.keys).to eq post_safe_hash.keys + excluded_keys end end diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 4f64f2bd6b4..1b7fa11cb3c 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -1,5 +1,7 @@ { "description": "Nisi et repellendus ut enim quo accusamus vel magnam.", + "import_type": "gitlab_project", + "creator_id": 123, "visibility_level": 10, "archived": false, "labels": [ diff --git a/spec/lib/gitlab/import_export/project.light.json b/spec/lib/gitlab/import_export/project.light.json index 5dbf0ed289b..c13cf4a0507 100644 --- a/spec/lib/gitlab/import_export/project.light.json +++ b/spec/lib/gitlab/import_export/project.light.json @@ -1,5 +1,7 @@ { "description": "Nisi et repellendus ut enim quo accusamus vel magnam.", + "import_type": "gitlab_project", + "creator_id": 123, "visibility_level": 10, "archived": false, "milestones": [ diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 13a8c9adcee..68ddc947e02 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -23,6 +23,10 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do allow_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch) project_tree_restorer = described_class.new(user: @user, shared: @shared, project: @project) + + expect(Gitlab::ImportExport::RelationFactory).to receive(:create).with(hash_including(excluded_keys: ['whatever'])).and_call_original.at_least(:once) + allow(project_tree_restorer).to receive(:excluded_keys_for_relation).and_return(['whatever']) + @restored_project_json = project_tree_restorer.restore end end @@ -248,6 +252,11 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect(labels.where(type: "ProjectLabel").count).to eq(results.fetch(:first_issue_labels, 0)) expect(labels.where(type: "ProjectLabel").where.not(group_id: nil).count).to eq(0) end + + it 'does not set params that are excluded from import_export settings' do + expect(project.import_type).to be_nil + expect(project.creator_id).not_to eq 123 + end end shared_examples 'restores group correctly' do |**results| diff --git a/spec/lib/gitlab/import_export/relation_factory_spec.rb b/spec/lib/gitlab/import_export/relation_factory_spec.rb index 5c61a5a2044..5f0dfd64b15 100644 --- a/spec/lib/gitlab/import_export/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/relation_factory_spec.rb @@ -4,12 +4,14 @@ describe Gitlab::ImportExport::RelationFactory do let(:project) { create(:project) } let(:members_mapper) { double('members_mapper').as_null_object } let(:user) { create(:admin) } + let(:excluded_keys) { [] } let(:created_object) do described_class.create(relation_sym: relation_sym, relation_hash: relation_hash, members_mapper: members_mapper, user: user, - project: project) + project: project, + excluded_keys: excluded_keys) end context 'hook object' do @@ -67,6 +69,14 @@ describe Gitlab::ImportExport::RelationFactory do expect(created_object.service_id).not_to eq(service_id) end end + + context 'excluded attributes' do + let(:excluded_keys) { %w[url] } + + it 'are removed from the imported object' do + expect(created_object.url).to be_nil + end + end end # Mocks an ActiveRecordish object with the dodgy columns diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 62da967cf96..3d5271cd030 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -165,6 +165,7 @@ MergeRequest: - approvals_before_merge - rebase_commit_sha - time_estimate +- squash - last_edited_at - last_edited_by_id - head_pipeline_id diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 7e47043a1cb..f8f07205623 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -391,68 +391,6 @@ describe ApplicationSetting do end describe 'performance bar settings' do - describe 'performance_bar_allowed_group_id=' do - context 'with a blank path' do - before do - setting.performance_bar_allowed_group_id = create(:group).full_path - end - - it 'persists nil for a "" path and clears allowed user IDs cache' do - expect(Gitlab::PerformanceBar).to receive(:expire_allowed_user_ids_cache) - - setting.performance_bar_allowed_group_id = '' - - expect(setting.performance_bar_allowed_group_id).to be_nil - end - end - - context 'with an invalid path' do - it 'does not persist an invalid group path' do - setting.performance_bar_allowed_group_id = 'foo' - - expect(setting.performance_bar_allowed_group_id).to be_nil - end - end - - context 'with a path to an existing group' do - let(:group) { create(:group) } - - it 'persists a valid group path and clears allowed user IDs cache' do - expect(Gitlab::PerformanceBar).to receive(:expire_allowed_user_ids_cache) - - setting.performance_bar_allowed_group_id = group.full_path - - expect(setting.performance_bar_allowed_group_id).to eq(group.id) - end - - context 'when the given path is the same' do - context 'with a blank path' do - before do - setting.performance_bar_allowed_group_id = nil - end - - it 'clears the cached allowed user IDs' do - expect(Gitlab::PerformanceBar).not_to receive(:expire_allowed_user_ids_cache) - - setting.performance_bar_allowed_group_id = '' - end - end - - context 'with a valid path' do - before do - setting.performance_bar_allowed_group_id = group.full_path - end - - it 'clears the cached allowed user IDs' do - expect(Gitlab::PerformanceBar).not_to receive(:expire_allowed_user_ids_cache) - - setting.performance_bar_allowed_group_id = group.full_path - end - end - end - end - end - describe 'performance_bar_allowed_group' do context 'with no performance_bar_allowed_group_id saved' do it 'returns nil' do @@ -464,11 +402,11 @@ describe ApplicationSetting do let(:group) { create(:group) } before do - setting.performance_bar_allowed_group_id = group.full_path + setting.update!(performance_bar_allowed_group_id: group.id) end it 'returns the group' do - expect(setting.performance_bar_allowed_group).to eq(group) + expect(setting.reload.performance_bar_allowed_group).to eq(group) end end end @@ -478,67 +416,11 @@ describe ApplicationSetting do let(:group) { create(:group) } before do - setting.performance_bar_allowed_group_id = group.full_path + setting.update!(performance_bar_allowed_group_id: group.id) end it 'returns true' do - expect(setting.performance_bar_enabled).to be_truthy - end - end - end - - describe 'performance_bar_enabled=' do - context 'when the performance bar is enabled' do - let(:group) { create(:group) } - - before do - setting.performance_bar_allowed_group_id = group.full_path - end - - context 'when passing true' do - it 'does not clear allowed user IDs cache' do - expect(Gitlab::PerformanceBar).not_to receive(:expire_allowed_user_ids_cache) - - setting.performance_bar_enabled = true - - expect(setting.performance_bar_allowed_group_id).to eq(group.id) - expect(setting.performance_bar_enabled).to be_truthy - end - end - - context 'when passing false' do - it 'disables the performance bar and clears allowed user IDs cache' do - expect(Gitlab::PerformanceBar).to receive(:expire_allowed_user_ids_cache) - - setting.performance_bar_enabled = false - - expect(setting.performance_bar_allowed_group_id).to be_nil - expect(setting.performance_bar_enabled).to be_falsey - end - end - end - - context 'when the performance bar is disabled' do - context 'when passing true' do - it 'does nothing and does not clear allowed user IDs cache' do - expect(Gitlab::PerformanceBar).not_to receive(:expire_allowed_user_ids_cache) - - setting.performance_bar_enabled = true - - expect(setting.performance_bar_allowed_group_id).to be_nil - expect(setting.performance_bar_enabled).to be_falsey - end - end - - context 'when passing false' do - it 'does nothing and does not clear allowed user IDs cache' do - expect(Gitlab::PerformanceBar).not_to receive(:expire_allowed_user_ids_cache) - - setting.performance_bar_enabled = false - - expect(setting.performance_bar_allowed_group_id).to be_nil - expect(setting.performance_bar_enabled).to be_falsey - end + expect(setting.reload.performance_bar_enabled).to be_truthy end end end diff --git a/spec/models/concerns/batch_destroy_dependent_associations_spec.rb b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb new file mode 100644 index 00000000000..c16b245bea8 --- /dev/null +++ b/spec/models/concerns/batch_destroy_dependent_associations_spec.rb @@ -0,0 +1,60 @@ +require 'spec_helper' + +describe BatchDestroyDependentAssociations do + class TestProject < ActiveRecord::Base + self.table_name = 'projects' + + has_many :builds, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_many :notification_settings, as: :source, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent + has_many :pages_domains + has_many :todos + + include BatchDestroyDependentAssociations + end + + describe '#dependent_associations_to_destroy' do + set(:project) { TestProject.new } + + it 'returns the right associations' do + expect(project.dependent_associations_to_destroy.map(&:name)).to match_array([:builds]) + end + end + + describe '#destroy_dependent_associations_in_batches' do + set(:project) { create(:project) } + set(:build) { create(:ci_build, project: project) } + set(:notification_setting) { create(:notification_setting, project: project) } + let!(:todos) { create(:todo, project: project) } + + it 'destroys multiple builds' do + create(:ci_build, project: project) + + expect(Ci::Build.count).to eq(2) + + project.destroy_dependent_associations_in_batches + + expect(Ci::Build.count).to eq(0) + end + + it 'destroys builds in batches' do + expect(project).to receive_message_chain(:builds, :find_each).and_yield(build) + expect(build).to receive(:destroy).and_call_original + + project.destroy_dependent_associations_in_batches + + expect(Ci::Build.count).to eq(0) + expect(Todo.count).to eq(1) + expect(User.count).to be > 0 + expect(NotificationSetting.count).to eq(User.count) + end + + it 'excludes associations' do + project.destroy_dependent_associations_in_batches(exclude: [:builds]) + + expect(Ci::Build.count).to eq(1) + expect(Todo.count).to eq(1) + expect(User.count).to be > 0 + expect(NotificationSetting.count).to eq(User.count) + end + end +end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 8ea92410022..c1eac4fa489 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -50,6 +50,19 @@ describe Event do end end + describe '#set_last_repository_updated_at' do + it 'only updates once every Event::REPOSITORY_UPDATED_AT_INTERVAL minutes' do + last_known_timestamp = (Event::REPOSITORY_UPDATED_AT_INTERVAL - 1.minute).ago + project.update(last_repository_updated_at: last_known_timestamp) + project.reload # a reload removes fractions of seconds + + expect do + create_push_event(project, project.owner) + project.reload + end.not_to change { project.last_repository_updated_at } + end + end + describe 'after_create :track_user_interacted_projects' do let(:event) { build(:push_event, project: project, author: project.owner) } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 92e33a64d26..9ffa91fc265 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -14,6 +14,65 @@ describe MergeRequest do it { is_expected.to have_many(:merge_request_diffs) } end + describe '#squash_in_progress?' do + shared_examples 'checking whether a squash is in progress' do + let(:repo_path) { subject.source_project.repository.path } + let(:squash_path) { File.join(repo_path, "gitlab-worktree", "squash-#{subject.id}") } + + before do + system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{squash_path} master)) + end + + it 'returns true when there is a current squash directory' do + expect(subject.squash_in_progress?).to be_truthy + end + + it 'returns false when there is no squash directory' do + FileUtils.rm_rf(squash_path) + + expect(subject.squash_in_progress?).to be_falsey + end + + it 'returns false when the squash directory has expired' do + time = 20.minutes.ago.to_time + File.utime(time, time, squash_path) + + expect(subject.squash_in_progress?).to be_falsey + end + + it 'returns false when the source project has been removed' do + allow(subject).to receive(:source_project).and_return(nil) + + expect(subject.squash_in_progress?).to be_falsey + end + end + + context 'when Gitaly squash_in_progress is enabled' do + it_behaves_like 'checking whether a squash is in progress' + end + + context 'when Gitaly squash_in_progress is disabled', :disable_gitaly do + it_behaves_like 'checking whether a squash is in progress' + end + end + + describe '#squash?' do + let(:merge_request) { build(:merge_request, squash: squash) } + subject { merge_request.squash? } + + context 'disabled in database' do + let(:squash) { false } + + it { is_expected.to be_falsy } + end + + context 'enabled in database' do + let(:squash) { true } + + it { is_expected.to be_truthy } + end + end + describe 'modules' do subject { described_class } diff --git a/spec/requests/api/deploy_keys_spec.rb b/spec/requests/api/deploy_keys_spec.rb index ae9c0e9c304..32fc704a79b 100644 --- a/spec/requests/api/deploy_keys_spec.rb +++ b/spec/requests/api/deploy_keys_spec.rb @@ -171,7 +171,7 @@ describe API::DeployKeys do deploy_key end - it 'deletes existing key' do + it 'removes existing key from project' do expect do delete api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin) @@ -179,6 +179,44 @@ describe API::DeployKeys do end.to change { project.deploy_keys.count }.by(-1) end + context 'when the deploy key is public' do + it 'does not delete the deploy key' do + expect do + delete api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin) + + expect(response).to have_gitlab_http_status(204) + end.not_to change { DeployKey.count } + end + end + + context 'when the deploy key is not public' do + let!(:deploy_key) { create(:deploy_key, public: false) } + + context 'when the deploy key is only used by this project' do + it 'deletes the deploy key' do + expect do + delete api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin) + + expect(response).to have_gitlab_http_status(204) + end.to change { DeployKey.count }.by(-1) + end + end + + context 'when the deploy key is used by other projects' do + before do + create(:deploy_keys_project, project: project2, deploy_key: deploy_key) + end + + it 'does not delete the deploy key' do + expect do + delete api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin) + + expect(response).to have_gitlab_http_status(204) + end.not_to change { DeployKey.count } + end + end + end + it 'returns 404 Not Found with invalid ID' do delete api("/projects/#{project.id}/deploy_keys/404", admin) diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 1eeeb4f1045..6b91e48ae6a 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -263,6 +263,7 @@ describe API::MergeRequests do expect(json_response.first['sha']).to eq(merge_request_merged.diff_head_sha) expect(json_response.first['merge_commit_sha']).not_to be_nil expect(json_response.first['merge_commit_sha']).to eq(merge_request_merged.merge_commit_sha) + expect(json_response.first['squash']).to eq(merge_request_merged.squash) end it "returns an array of all merge_requests using simple mode" do @@ -671,12 +672,14 @@ describe API::MergeRequests do target_branch: 'master', author: user, labels: 'label, label2', - milestone_id: milestone.id + milestone_id: milestone.id, + squash: true expect(response).to have_gitlab_http_status(201) expect(json_response['title']).to eq('Test merge_request') expect(json_response['labels']).to eq(%w(label label2)) expect(json_response['milestone']['id']).to eq(milestone.id) + expect(json_response['squash']).to be_truthy expect(json_response['force_remove_source_branch']).to be_falsy end @@ -965,6 +968,14 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(200) end + it "updates the MR's squash attribute" do + expect do + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), squash: true + end.to change { merge_request.reload.squash } + + expect(response).to have_gitlab_http_status(200) + end + it "enables merge when pipeline succeeds if the pipeline is active" do allow_any_instance_of(MergeRequest).to receive(:head_pipeline).and_return(pipeline) allow(pipeline).to receive(:active?).and_return(true) @@ -1029,6 +1040,13 @@ describe API::MergeRequests do expect(json_response['milestone']['id']).to eq(milestone.id) end + it "updates squash and returns merge_request" do + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), squash: true + + expect(response).to have_gitlab_http_status(200) + expect(json_response['squash']).to be_truthy + end + it "returns merge_request with renamed target_branch" do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), target_branch: "wiki" expect(response).to have_gitlab_http_status(200) diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 8b22d1e72f3..aead8978dd4 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -24,10 +24,15 @@ describe API::Settings, 'Settings' do expect(json_response['ecdsa_key_restriction']).to eq(0) expect(json_response['ed25519_key_restriction']).to eq(0) expect(json_response['circuitbreaker_failure_count_threshold']).not_to be_nil + expect(json_response['performance_bar_allowed_group_id']).to be_nil + expect(json_response).not_to have_key('performance_bar_allowed_group_path') + expect(json_response).not_to have_key('performance_bar_enabled') end end describe "PUT /application/settings" do + let(:group) { create(:group) } + context "custom repository storage type set in the config" do before do storages = { 'custom' => 'tmp/tests/custom_repositories' } @@ -56,7 +61,8 @@ describe API::Settings, 'Settings' do ed25519_key_restriction: 256, circuitbreaker_check_interval: 2, enforce_terms: true, - terms: 'Hello world!' + terms: 'Hello world!', + performance_bar_allowed_group_path: group.full_path expect(response).to have_gitlab_http_status(200) expect(json_response['default_projects_limit']).to eq(3) @@ -80,9 +86,27 @@ describe API::Settings, 'Settings' do expect(json_response['circuitbreaker_check_interval']).to eq(2) expect(json_response['enforce_terms']).to be(true) expect(json_response['terms']).to eq('Hello world!') + expect(json_response['performance_bar_allowed_group_id']).to eq(group.id) end end + it "supports legacy performance_bar_allowed_group_id" do + put api("/application/settings", admin), + performance_bar_allowed_group_id: group.full_path + + expect(response).to have_gitlab_http_status(200) + expect(json_response['performance_bar_allowed_group_id']).to eq(group.id) + end + + it "supports legacy performance_bar_enabled" do + put api("/application/settings", admin), + performance_bar_enabled: false, + performance_bar_allowed_group_id: group.full_path + + expect(response).to have_gitlab_http_status(200) + expect(json_response['performance_bar_allowed_group_id']).to be_nil + end + context "missing koding_url value when koding_enabled is true" do it "returns a blank parameter error message" do put api("/application/settings", admin), koding_enabled: true diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb index be70cb24dce..79a16fbd1b0 100644 --- a/spec/requests/api/v3/merge_requests_spec.rb +++ b/spec/requests/api/v3/merge_requests_spec.rb @@ -40,6 +40,7 @@ describe API::MergeRequests do expect(json_response.first['sha']).to eq(merge_request_merged.diff_head_sha) expect(json_response.first['merge_commit_sha']).not_to be_nil expect(json_response.first['merge_commit_sha']).to eq(merge_request_merged.merge_commit_sha) + expect(json_response.first['squash']).to eq(merge_request_merged.squash) end it "returns an array of all merge_requests" do @@ -241,13 +242,15 @@ describe API::MergeRequests do author: user, labels: 'label, label2', milestone_id: milestone.id, - remove_source_branch: true + remove_source_branch: true, + squash: true expect(response).to have_gitlab_http_status(201) expect(json_response['title']).to eq('Test merge_request') expect(json_response['labels']).to eq(%w(label label2)) expect(json_response['milestone']['id']).to eq(milestone.id) expect(json_response['force_remove_source_branch']).to be_truthy + expect(json_response['squash']).to be_truthy end it "returns 422 when source_branch equals target_branch" do @@ -489,6 +492,14 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(200) end + it "updates the MR's squash attribute" do + expect do + put v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), squash: true + end.to change { merge_request.reload.squash } + + expect(response).to have_gitlab_http_status(200) + end + it "enables merge when pipeline succeeds if the pipeline is active" do allow_any_instance_of(MergeRequest).to receive(:head_pipeline).and_return(pipeline) allow(pipeline).to receive(:active?).and_return(true) @@ -529,6 +540,13 @@ describe API::MergeRequests do expect(json_response['milestone']['id']).to eq(milestone.id) end + it "updates squash and returns merge_request" do + put v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), squash: true + + expect(response).to have_gitlab_http_status(200) + expect(json_response['squash']).to be_truthy + end + it "returns merge_request with renamed target_branch" do put v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), target_branch: "wiki" expect(response).to have_gitlab_http_status(200) diff --git a/spec/rubocop/cop/line_break_around_conditional_block_spec.rb b/spec/rubocop/cop/line_break_around_conditional_block_spec.rb index 7ddf9141fcd..03eeffe6483 100644 --- a/spec/rubocop/cop/line_break_around_conditional_block_spec.rb +++ b/spec/rubocop/cop/line_break_around_conditional_block_spec.rb @@ -256,6 +256,18 @@ describe RuboCop::Cop::LineBreakAroundConditionalBlock do expect(cop.offenses).to be_empty end + it "doesn't flag violation for #{conditional} followed by a comment" do + source = <<~RUBY + #{conditional} condition + do_something + end + # a short comment + RUBY + inspect_source(source) + + expect(cop.offenses).to be_empty + end + it "doesn't flag violation for #{conditional} followed by an end" do source = <<~RUBY class Foo diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb index fb07ecc6ae8..6337ee7d724 100644 --- a/spec/services/application_settings/update_service_spec.rb +++ b/spec/services/application_settings/update_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe ApplicationSettings::UpdateService do - let(:application_settings) { Gitlab::CurrentSettings.current_application_settings } + let(:application_settings) { create(:application_setting) } let(:admin) { create(:user, :admin) } let(:params) { {} } @@ -54,4 +54,90 @@ describe ApplicationSettings::UpdateService do end end end + + describe 'performance bar settings' do + using RSpec::Parameterized::TableSyntax + + where(:params_performance_bar_enabled, + :params_performance_bar_allowed_group_path, + :previous_performance_bar_allowed_group_id, + :expected_performance_bar_allowed_group_id) do + true | '' | nil | nil + true | '' | 42_000_000 | nil + true | nil | nil | nil + true | nil | 42_000_000 | nil + true | 'foo' | nil | nil + true | 'foo' | 42_000_000 | nil + true | 'group_a' | nil | 42_000_000 + true | 'group_b' | 42_000_000 | 43_000_000 + true | 'group_a' | 42_000_000 | 42_000_000 + false | '' | nil | nil + false | '' | 42_000_000 | nil + false | nil | nil | nil + false | nil | 42_000_000 | nil + false | 'foo' | nil | nil + false | 'foo' | 42_000_000 | nil + false | 'group_a' | nil | nil + false | 'group_b' | 42_000_000 | nil + false | 'group_a' | 42_000_000 | nil + end + + with_them do + let(:params) do + { + performance_bar_enabled: params_performance_bar_enabled, + performance_bar_allowed_group_path: params_performance_bar_allowed_group_path + } + end + + before do + if previous_performance_bar_allowed_group_id == 42_000_000 || params_performance_bar_allowed_group_path == 'group_a' + create(:group, id: 42_000_000, path: 'group_a') + end + + if expected_performance_bar_allowed_group_id == 43_000_000 || params_performance_bar_allowed_group_path == 'group_b' + create(:group, id: 43_000_000, path: 'group_b') + end + + application_settings.update!(performance_bar_allowed_group_id: previous_performance_bar_allowed_group_id) + end + + it 'sets performance_bar_allowed_group_id when present and performance_bar_enabled == true' do + expect(application_settings.performance_bar_allowed_group_id).to eq(previous_performance_bar_allowed_group_id) + + if previous_performance_bar_allowed_group_id != expected_performance_bar_allowed_group_id + expect { subject.execute } + .to change(application_settings, :performance_bar_allowed_group_id) + .from(previous_performance_bar_allowed_group_id).to(expected_performance_bar_allowed_group_id) + else + expect { subject.execute } + .not_to change(application_settings, :performance_bar_allowed_group_id) + end + end + end + + context 'when :performance_bar_allowed_group_path is not present' do + let(:group) { create(:group) } + + before do + application_settings.update!(performance_bar_allowed_group_id: group.id) + end + + it 'does not change the performance bar settings' do + expect { subject.execute } + .not_to change(application_settings, :performance_bar_allowed_group_id) + end + end + + context 'when :performance_bar_enabled is not present' do + let(:group) { create(:group) } + let(:params) { { performance_bar_allowed_group_path: group.full_path } } + + it 'implicitely defaults to true' do + expect { subject.execute } + .to change(application_settings, :performance_bar_allowed_group_id) + .from(nil).to(group.id) + end + end + end end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index e8568bf8bb3..dc30a9bccc1 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -249,24 +249,58 @@ describe MergeRequests::MergeService do expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) end - context "when fast-forward merge is not allowed" do + context 'when squashing' do before do - allow_any_instance_of(Repository).to receive(:ancestor?).and_return(nil) + merge_request.update!(source_branch: 'master', target_branch: 'feature') end - %w(semi-linear ff).each do |merge_method| - it "logs and saves error if merge is #{merge_method} only" do - merge_method = 'rebase_merge' if merge_method == 'semi-linear' - merge_request.project.update(merge_method: merge_method) - error_message = 'Only fast-forward merge is allowed for your project. Please update your source branch' - allow(service).to receive(:execute_hooks) + it 'logs and saves error if there is an error when squashing' do + error_message = 'Failed to squash. Should be done manually' - service.execute(merge_request) + allow_any_instance_of(MergeRequests::SquashService).to receive(:squash).and_return(nil) + merge_request.update(squash: true) + + service.execute(merge_request) + + expect(merge_request).to be_open + expect(merge_request.merge_commit_sha).to be_nil + expect(merge_request.merge_error).to include(error_message) + expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + end + + it 'logs and saves error if there is a squash in progress' do + error_message = 'another squash is already in progress' + + allow_any_instance_of(MergeRequest).to receive(:squash_in_progress?).and_return(true) + merge_request.update(squash: true) + + service.execute(merge_request) - expect(merge_request).to be_open - expect(merge_request.merge_commit_sha).to be_nil - expect(merge_request.merge_error).to include(error_message) - expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + expect(merge_request).to be_open + expect(merge_request.merge_commit_sha).to be_nil + expect(merge_request.merge_error).to include(error_message) + expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + end + + context "when fast-forward merge is not allowed" do + before do + allow_any_instance_of(Repository).to receive(:ancestor?).and_return(nil) + end + + %w(semi-linear ff).each do |merge_method| + it "logs and saves error if merge is #{merge_method} only" do + merge_method = 'rebase_merge' if merge_method == 'semi-linear' + merge_request.project.update(merge_method: merge_method) + error_message = 'Only fast-forward merge is allowed for your project. Please update your source branch' + allow(service).to receive(:execute_hooks) + + service.execute(merge_request) + + expect(merge_request).to be_open + expect(merge_request.merge_commit_sha).to be_nil + expect(merge_request.merge_error).to include(error_message) + expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + end end end end diff --git a/spec/services/merge_requests/squash_service_spec.rb b/spec/services/merge_requests/squash_service_spec.rb new file mode 100644 index 00000000000..bd884787425 --- /dev/null +++ b/spec/services/merge_requests/squash_service_spec.rb @@ -0,0 +1,199 @@ +require 'spec_helper' + +describe MergeRequests::SquashService do + let(:service) { described_class.new(project, user, {}) } + let(:user) { project.owner } + let(:project) { create(:project, :repository) } + let(:repository) { project.repository.raw } + let(:log_error) { "Failed to squash merge request #{merge_request.to_reference(full: true)}:" } + let(:squash_dir_path) do + File.join(Gitlab.config.shared.path, 'tmp/squash', repository.gl_repository, merge_request.id.to_s) + end + let(:merge_request_with_one_commit) do + create(:merge_request, + source_branch: 'feature', source_project: project, + target_branch: 'master', target_project: project) + end + + let(:merge_request_with_only_new_files) do + create(:merge_request, + source_branch: 'video', source_project: project, + target_branch: 'master', target_project: project) + end + + let(:merge_request_with_large_files) do + create(:merge_request, + source_branch: 'squash-large-files', source_project: project, + target_branch: 'master', target_project: project) + end + + shared_examples 'the squash succeeds' do + it 'returns the squashed commit SHA' do + result = service.execute(merge_request) + + expect(result).to match(status: :success, squash_sha: a_string_matching(/\h{40}/)) + expect(result[:squash_sha]).not_to eq(merge_request.diff_head_sha) + end + + it 'cleans up the temporary directory' do + service.execute(merge_request) + + expect(File.exist?(squash_dir_path)).to be(false) + end + + it 'does not keep the branch push event' do + expect { service.execute(merge_request) }.not_to change { Event.count } + end + + context 'the squashed commit' do + let(:squash_sha) { service.execute(merge_request)[:squash_sha] } + let(:squash_commit) { project.repository.commit(squash_sha) } + + it 'copies the author info and message from the merge request' do + expect(squash_commit.author_name).to eq(merge_request.author.name) + expect(squash_commit.author_email).to eq(merge_request.author.email) + + # Commit messages have a trailing newline, but titles don't. + expect(squash_commit.message.chomp).to eq(merge_request.title) + end + + it 'sets the current user as the committer' do + expect(squash_commit.committer_name).to eq(user.name.chomp('.')) + expect(squash_commit.committer_email).to eq(user.email) + end + + it 'has the same diff as the merge request, but a different SHA' do + rugged = project.repository.rugged + mr_diff = rugged.diff(merge_request.diff_base_sha, merge_request.diff_head_sha) + squash_diff = rugged.diff(merge_request.diff_start_sha, squash_sha) + + expect(squash_diff.patch.length).to eq(mr_diff.patch.length) + expect(squash_commit.sha).not_to eq(merge_request.diff_head_sha) + end + end + end + + describe '#execute' do + context 'when there is only one commit in the merge request' do + it 'returns that commit SHA' do + result = service.execute(merge_request_with_one_commit) + + expect(result).to match(status: :success, squash_sha: merge_request_with_one_commit.diff_head_sha) + end + + it 'does not perform any git actions' do + expect(repository).not_to receive(:popen) + + service.execute(merge_request_with_one_commit) + end + end + + context 'when squashing only new files' do + let(:merge_request) { merge_request_with_only_new_files } + + include_examples 'the squash succeeds' + end + + context 'when squashing with files too large to display' do + let(:merge_request) { merge_request_with_large_files } + + include_examples 'the squash succeeds' + end + + context 'git errors' do + let(:merge_request) { merge_request_with_only_new_files } + let(:error) { 'A test error' } + + context 'with gitaly enabled' do + before do + allow(repository.gitaly_operation_client).to receive(:user_squash) + .and_raise(Gitlab::Git::Repository::GitError, error) + end + + it 'logs the stage and output' do + expect(service).to receive(:log_error).with(log_error) + expect(service).to receive(:log_error).with(error) + + service.execute(merge_request) + end + + it 'returns an error' do + expect(service.execute(merge_request)).to match(status: :error, + message: a_string_including('squash')) + end + end + + context 'with Gitaly disabled', :skip_gitaly_mock do + stages = { + 'add worktree for squash' => 'worktree', + 'configure sparse checkout' => 'config', + 'get files in diff' => 'diff --name-only', + 'check out target branch' => 'checkout', + 'apply patch' => 'diff --binary', + 'commit squashed changes' => 'commit', + 'get SHA of squashed commit' => 'rev-parse' + } + + stages.each do |stage, command| + context "when the #{stage} stage fails" do + before do + git_command = a_collection_containing_exactly( + a_string_starting_with("#{Gitlab.config.git.bin_path} #{command}") + ).or( + a_collection_starting_with([Gitlab.config.git.bin_path] + command.split) + ) + + allow(repository).to receive(:popen).and_return(['', 0]) + allow(repository).to receive(:popen).with(git_command, anything, anything, anything).and_return([error, 1]) + end + + it 'logs the stage and output' do + expect(service).to receive(:log_error).with(log_error) + expect(service).to receive(:log_error).with(error) + + service.execute(merge_request) + end + + it 'returns an error' do + expect(service.execute(merge_request)).to match(status: :error, + message: a_string_including('squash')) + end + + it 'cleans up the temporary directory' do + expect(File.exist?(squash_dir_path)).to be(false) + + service.execute(merge_request) + end + end + end + end + end + + context 'when any other exception is thrown' do + let(:merge_request) { merge_request_with_only_new_files } + let(:error) { 'A test error' } + + before do + allow(merge_request).to receive(:commits_count).and_raise(error) + end + + it 'logs the MR reference and exception' do + expect(service).to receive(:log_error).with(a_string_including("#{project.full_path}#{merge_request.to_reference}")) + expect(service).to receive(:log_error).with(error) + + service.execute(merge_request) + end + + it 'returns an error' do + expect(service.execute(merge_request)).to match(status: :error, + message: a_string_including('squash')) + end + + it 'cleans up the temporary directory' do + service.execute(merge_request) + + expect(File.exist?(squash_dir_path)).to be(false) + end + end + end +end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index 57aa07cf4fa..1fef50a52ec 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -47,6 +47,7 @@ module TestEnv 'v1.1.0' => 'b83d6e3', 'add-ipython-files' => '93ee732', 'add-pdf-file' => 'e774ebd', + 'squash-large-files' => '54cec52', 'add-pdf-text-binary' => '79faa7b', 'add_images_and_changes' => '010d106' }.freeze |