diff options
175 files changed, 1838 insertions, 678 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e78615e3c29..295f9bca24b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -349,7 +349,7 @@ on those issues. Please select someone with relevant experience from the [GitLab team][team]. If there is nobody mentioned with that expertise look in the commit history for the affected files to find someone. -[described in our handbook]: https://about.gitlab.com/handbook/engineering/issues/issue-triage-policies/ +[described in our handbook]: https://about.gitlab.com/handbook/engineering/issue-triage/ [issue bash events]: https://gitlab.com/gitlab-org/gitlab-ce/issues/17815 ### Feature proposals diff --git a/PROCESS.md b/PROCESS.md index f206506f7c5..7438df8014b 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -168,6 +168,7 @@ the stable branch are: * Fixes for [regressions](#regressions) * Fixes for security issues +* Fixes or improvements to automated QA scenarios * New or updated translations (as long as they do not touch application code) During the feature freeze all merge requests that are meant to go into the diff --git a/app/assets/javascripts/importer_status.js b/app/assets/javascripts/importer_status.js index b469e1e2adc..52455885248 100644 --- a/app/assets/javascripts/importer_status.js +++ b/app/assets/javascripts/importer_status.js @@ -58,7 +58,7 @@ class ImporterStatus { job.find('.import-target').html(`<a href="${data.full_path}">${data.full_path}</a>`); $('table.import-jobs tbody').prepend(job); - job.addClass('active'); + job.addClass('table-active'); const connectingVerb = this.ciCdOnly ? __('connecting') : __('importing'); job.find('.import-actions').html(sprintf( _.escape(__('%{loadingIcon} Started')), { @@ -81,7 +81,7 @@ class ImporterStatus { switch (job.import_status) { case 'finished': - jobItem.removeClass('active').addClass('success'); + jobItem.removeClass('table-active').addClass('table-success'); statusField.html(`<span><i class="fa fa-check"></i> ${__('Done')}</span>`); break; case 'scheduled': diff --git a/app/assets/javascripts/performance_bar/components/detailed_metric.vue b/app/assets/javascripts/performance_bar/components/detailed_metric.vue index db8a0055acd..96189e7033a 100644 --- a/app/assets/javascripts/performance_bar/components/detailed_metric.vue +++ b/app/assets/javascripts/performance_bar/components/detailed_metric.vue @@ -56,6 +56,7 @@ export default { <gl-modal :id="`modal-peek-${metric}-details`" :header-title-text="header" + modal-size="lg" class="performance-bar-modal" > <table @@ -70,7 +71,7 @@ export default { <td v-for="key in keys" :key="key" - class="break-word" + class="break-word all-words" > {{ item[key] }} </td> diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue index f69fe03fcb3..c20d07a169d 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue @@ -265,10 +265,10 @@ export default { /> <section - v-if="mr.maintainerEditAllowed" + v-if="mr.allowCollaboration" class="mr-info-list mr-links" > - {{ s__("mrWidget|Allows edits from maintainers") }} + {{ s__("mrWidget|Allows commits from members who can merge to the target branch") }} </section> <mr-widget-related-links 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 e5b7e1f1c68..134aaacf9d2 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 @@ -83,7 +83,7 @@ export default class MergeRequestStore { this.canBeMerged = data.can_be_merged || false; this.isMergeAllowed = data.mergeable || false; this.mergeOngoing = data.merge_ongoing; - this.maintainerEditAllowed = data.allow_maintainer_to_push; + this.allowCollaboration = data.allow_collaboration; // Cherry-pick and Revert actions related this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false; diff --git a/app/assets/javascripts/vue_shared/components/gl_modal.vue b/app/assets/javascripts/vue_shared/components/gl_modal.vue index d5d5a7d3798..7ba58bd5959 100644 --- a/app/assets/javascripts/vue_shared/components/gl_modal.vue +++ b/app/assets/javascripts/vue_shared/components/gl_modal.vue @@ -1,15 +1,21 @@ <script> const buttonVariants = ['danger', 'primary', 'success', 'warning']; +const sizeVariants = ['sm', 'md', 'lg']; export default { name: 'GlModal', - props: { id: { type: String, required: false, default: null, }, + modalSize: { + type: String, + required: false, + default: 'md', + validator: value => sizeVariants.includes(value), + }, headerTitleText: { type: String, required: false, @@ -27,7 +33,11 @@ export default { default: '', }, }, - + computed: { + modalSizeClass() { + return this.modalSize === 'md' ? '' : `modal-${this.modalSize}`; + }, + }, methods: { emitCancel(event) { this.$emit('cancel', event); @@ -48,6 +58,7 @@ export default { > <div class="modal-dialog" + :class="modalSizeClass" role="document" > <div class="modal-content"> diff --git a/app/assets/javascripts/vue_shared/components/table_pagination.vue b/app/assets/javascripts/vue_shared/components/table_pagination.vue index 22fc5757447..6f231619f26 100644 --- a/app/assets/javascripts/vue_shared/components/table_pagination.vue +++ b/app/assets/javascripts/vue_shared/components/table_pagination.vue @@ -124,15 +124,18 @@ break; } }, + hideOnSmallScreen(item) { + return !item.first && !item.last && !item.next && !item.prev && !item.active; + }, }, }; </script> <template> <div v-if="showPagination" - class="gl-pagination" + class="gl-pagination prepend-top-default" > - <ul class="pagination clearfix"> + <ul class="pagination justify-content-center"> <li v-for="(item, index) in getItems" :key="index" @@ -142,12 +145,17 @@ 'js-next-button': item.next, 'js-last-button': item.last, 'js-first-button': item.first, + 'd-none d-md-block': hideOnSmallScreen(item), separator: item.separator, active: item.active, - disabled: item.disabled + disabled: item.disabled || item.separator }" + class="page-item" > - <a @click.prevent="changePage(item.title, item.disabled)"> + <a + @click.prevent="changePage(item.title, item.disabled)" + class="page-link" + > {{ item.title }} </a> </li> diff --git a/app/assets/stylesheets/bootstrap_migration.scss b/app/assets/stylesheets/bootstrap_migration.scss index d5679177f8f..3785aaa43f0 100644 --- a/app/assets/stylesheets/bootstrap_migration.scss +++ b/app/assets/stylesheets/bootstrap_migration.scss @@ -24,16 +24,54 @@ html { font-size: 14px; } +legend { + border-bottom: 1px solid $border-color; + margin-bottom: 20px; +} + button, html [type="button"], [type="reset"], -[type="submit"] { +[type="submit"], +[role="button"] { // Override bootstrap reboot -webkit-appearance: inherit; + cursor: pointer; } -[role="button"] { - cursor: pointer; +h1, +h2, +h3, +h4, +h5, +h6 { + color: $gl-text-color; + font-weight: 600; +} + +h1, +.h1, +h2, +.h2, +h3, +.h3 { + margin-top: 20px; + margin-bottom: 10px; +} + +h4, +.h4, +h5, +.h5, +h6, +.h6 { + margin-top: 10px; + margin-bottom: 10px; +} + +h5, +.h5 { + font-size: $gl-font-size; } input[type="file"] { @@ -59,6 +97,10 @@ a { } } +kbd { + display: inline-block; +} + code { padding: 2px 4px; color: $red-600; @@ -108,6 +150,16 @@ table { color: $gl-text-color-secondary !important; } +.bg-success, +.bg-primary, +.bg-info, +.bg-danger, +.bg-warning { + .card-header { + color: $white-light; + } +} + // Polyfill deprecated selectors .hidden { @@ -182,8 +234,13 @@ table { } .nav-tabs { + // Override bootstrap's default border + border-bottom: 0; + .nav-link { - border: 0; + border-top: 0; + border-left: 0; + border-right: 0; } .nav-item { diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 1e7b9534275..996e5c1512d 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -448,6 +448,10 @@ img.emoji { .break-word { word-wrap: break-word; + + &.all-words { + word-break: break-word; + } } /** COMMON CLASSES **/ diff --git a/app/assets/stylesheets/framework/pagination.scss b/app/assets/stylesheets/framework/pagination.scss index d3e013590b6..50a1b1c446d 100644 --- a/app/assets/stylesheets/framework/pagination.scss +++ b/app/assets/stylesheets/framework/pagination.scss @@ -1,91 +1,6 @@ .gl-pagination { - text-align: center; - border-top: 1px solid $border-color; - margin: 0; - margin-top: 0; - - .pagination { - padding: 0; - margin: 20px 0; - - a { - cursor: pointer; - } - - .separator, - .separator:hover { - a { - cursor: default; - background-color: $gray-light; - padding: $gl-vert-padding; - } - } - } - - - .gap, - .gap:hover { - background-color: $gray-light; - padding: $gl-vert-padding; - cursor: default; - } -} - -.card > .gl-pagination { - margin: 0; -} - -/** - * Extra-small screen pagination. - */ -@media (max-width: 320px) { - .gl-pagination { - .first, - .last { - display: none; - } - - .page-item { - display: none; - - &.active { - display: inline; - } - } - } -} - -/** - * Small screen pagination - */ -@include media-breakpoint-down(xs) { - .gl-pagination { - .pagination li a { - padding: 6px 10px; - } - - .page-item { - display: none; - - &.active { - display: inline; - } - } - } -} - -/** - * Medium screen pagination - */ -@media (min-width: map-get($grid-breakpoints, xs)) and (max-width: map-get($grid-breakpoints, sm)) { - .gl-pagination { - .page-item { - display: none; - - &.active, - &.sibling { - display: inline; - } - } + a { + color: inherit; + text-decoration: none; } } diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 4aea9740735..b42c232fd91 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -485,6 +485,15 @@ .sidebar-collapsed-user { padding-bottom: 0; margin-bottom: 10px; + + .author_link { + padding-left: 0; + + .avatar { + position: static; + margin: 0; + } + } } .issuable-header-btn { diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index 43c39c43bc1..16e999341da 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -102,10 +102,6 @@ .form-text.text-muted { margin-top: 0; } - - .label-light { - margin-bottom: 0; - } } .settings-list-icon { diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index db8a8cdc0d2..bc60a0a02e8 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -130,12 +130,17 @@ class ApplicationController < ActionController::Base end def access_denied!(message = nil) + # If we display a custom access denied message to the user, we don't want to + # hide existence of the resource, rather tell them they cannot access it using + # the provided message + status = message.present? ? :forbidden : :not_found + respond_to do |format| - format.any { head :not_found } + format.any { head status } format.html do render "errors/access_denied", layout: "errors", - status: 404, + status: status, locals: { message: message } end end diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index ef3eba80154..ef5d5e5c742 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -3,8 +3,12 @@ class Groups::GroupMembersController < Groups::ApplicationController include MembersPresentation include SortingHelper + def self.admin_not_required_endpoints + %i[index leave request_access] + end + # Authorize - before_action :authorize_admin_group_member!, except: [:index, :leave, :request_access] + before_action :authorize_admin_group_member!, except: admin_not_required_endpoints skip_cross_project_access_check :index, :create, :update, :destroy, :request_access, :approve_access_request, :leave, :resend_invite, diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb index 5903689dc62..9bd51de7e97 100644 --- a/app/controllers/groups/milestones_controller.rb +++ b/app/controllers/groups/milestones_controller.rb @@ -76,12 +76,15 @@ class Groups::MilestonesController < Groups::ApplicationController def milestones milestones = MilestonesFinder.new(search_params).execute - legacy_milestones = GroupMilestone.build_collection(group, group_projects, params) @sort = params[:sort] || 'due_date_asc' MilestoneArray.sort(milestones + legacy_milestones, @sort) end + def legacy_milestones + GroupMilestone.build_collection(group, group_projects, params) + end + def milestone @milestone = if params[:title] diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb index 29632bef7e5..8e4aeec16dc 100644 --- a/app/controllers/projects/merge_requests/application_controller.rb +++ b/app/controllers/projects/merge_requests/application_controller.rb @@ -15,7 +15,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont def merge_request_params_attributes [ - :allow_maintainer_to_push, + :allow_collaboration, :assignee_id, :description, :force_remove_source_branch, diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 6b40fc2fe68..768595ceeb4 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -23,8 +23,6 @@ class Projects::PipelinesController < Projects::ApplicationController @finished_count = limited_pipelines_count(project, 'finished') @pipelines_count = limited_pipelines_count(project) - Gitlab::Ci::Pipeline::Preloader.preload(@pipelines) - respond_to do |format| format.html format.json do @@ -34,7 +32,7 @@ class Projects::PipelinesController < Projects::ApplicationController pipelines: PipelineSerializer .new(project: @project, current_user: @current_user) .with_pagination(request, response) - .represent(@pipelines, disable_coverage: true), + .represent(@pipelines, disable_coverage: true, preload: true), count: { all: @pipelines_count, running: @running_count, diff --git a/app/controllers/users/terms_controller.rb b/app/controllers/users/terms_controller.rb index ab685b9106e..f7c6d1d59db 100644 --- a/app/controllers/users/terms_controller.rb +++ b/app/controllers/users/terms_controller.rb @@ -13,6 +13,10 @@ module Users def index @redirect = redirect_path + + if @term.accepted_by_user?(current_user) + flash.now[:notice] = "You have already accepted the Terms of Service as #{current_user.to_reference}" + end end def accept diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 74251c260f0..5ff06b3e0fc 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -126,8 +126,8 @@ module MergeRequestsHelper link_to(url[merge_request.project, merge_request], data: data_attrs, &block) end - def allow_maintainer_push_unavailable_reason(merge_request) - return if merge_request.can_allow_maintainer_to_push?(current_user) + def allow_collaboration_unavailable_reason(merge_request) + return if merge_request.can_allow_collaboration?(current_user) minimum_visibility = [merge_request.target_project.visibility_level, merge_request.source_project.visibility_level].min diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index dfca799a53d..cdbb572f80a 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -389,11 +389,11 @@ module ProjectsHelper def project_status_css_class(status) case status when "started" - "active" + "table-active" when "failed" - "danger" + "table-danger" when "finished" - "success" + "table-success" end end @@ -412,7 +412,10 @@ module ProjectsHelper exports_path = File.join(Settings.shared['path'], 'tmp/project_exports') filtered_message = message.strip.gsub(exports_path, "[REPO EXPORT PATH]") - disk_path = Gitlab.config.repositories.storages[project.repository_storage].legacy_disk_path + disk_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages[project.repository_storage].legacy_disk_path + end + filtered_message.gsub(disk_path.chomp('/'), "[REPOS PATH]") end diff --git a/app/helpers/workhorse_helper.rb b/app/helpers/workhorse_helper.rb index 9f78b80c71d..a82271ce0ee 100644 --- a/app/helpers/workhorse_helper.rb +++ b/app/helpers/workhorse_helper.rb @@ -6,7 +6,7 @@ module WorkhorseHelper headers.store(*Gitlab::Workhorse.send_git_blob(repository, blob)) headers['Content-Disposition'] = 'inline' headers['Content-Type'] = safe_content_type(blob) - head :ok # 'render nothing: true' messes up the Content-Type + render plain: "" end # Send a Git diff through Workhorse diff --git a/app/models/application_setting/term.rb b/app/models/application_setting/term.rb index e8ce0ccbb71..3b1dfe7e4ef 100644 --- a/app/models/application_setting/term.rb +++ b/app/models/application_setting/term.rb @@ -1,6 +1,7 @@ class ApplicationSetting class Term < ActiveRecord::Base include CacheMarkdownField + has_many :term_agreements validates :terms, presence: true @@ -9,5 +10,10 @@ class ApplicationSetting def self.latest order(:id).last end + + def accepted_by_user?(user) + user.accepted_term_id == id || + term_agreements.accepted.where(user: user).exists? + end end end diff --git a/app/models/ci/group.rb b/app/models/ci/group.rb index 87898b086c6..9c1046e8715 100644 --- a/app/models/ci/group.rb +++ b/app/models/ci/group.rb @@ -31,6 +31,14 @@ module Ci end end + def self.fabricate(stage) + stage.statuses.ordered.latest + .sort_by(&:sortable_name).group_by(&:group_name) + .map do |group_name, grouped_statuses| + self.new(stage, name: group_name, jobs: grouped_statuses) + end + end + private def commit_statuses diff --git a/app/models/ci/legacy_stage.rb b/app/models/ci/legacy_stage.rb index 9b536af672b..ce691875e42 100644 --- a/app/models/ci/legacy_stage.rb +++ b/app/models/ci/legacy_stage.rb @@ -16,11 +16,7 @@ module Ci end def groups - @groups ||= statuses.ordered.latest - .sort_by(&:sortable_name).group_by(&:group_name) - .map do |group_name, grouped_statuses| - Ci::Group.new(self, name: group_name, jobs: grouped_statuses) - end + @groups ||= Ci::Group.fabricate(self) end def to_param diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 5eb30f4aaa0..eecd86349e4 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -18,7 +18,7 @@ module Ci s&.project&.pipelines&.maximum(:iid) || s&.project&.pipelines&.count end - has_many :stages + has_many :stages, -> { order(position: :asc) }, inverse_of: :pipeline has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline has_many :builds, foreign_key: :commit_id, inverse_of: :pipeline has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id # rubocop:disable Cop/ActiveRecordDependent @@ -254,6 +254,20 @@ module Ci stage unless stage.statuses_count.zero? end + ## + # TODO We do not completely switch to persisted stages because of + # race conditions with setting statuses gitlab-ce#23257. + # + def ordered_stages + return legacy_stages unless complete? + + if Feature.enabled?('ci_pipeline_persisted_stages') + stages + else + legacy_stages + end + end + def legacy_stages # TODO, this needs refactoring, see gitlab-ce#26481. @@ -416,7 +430,7 @@ module Ci def number_of_warnings BatchLoader.for(id).batch(default_value: 0) do |pipeline_ids, loader| - Build.where(commit_id: pipeline_ids) + ::Ci::Build.where(commit_id: pipeline_ids) .latest .failed_but_allowed .group(:commit_id) @@ -508,7 +522,8 @@ module Ci def update_status retry_optimistic_lock(self) do - case latest_builds_status + case latest_builds_status.to_s + when 'created' then nil when 'pending' then enqueue when 'running' then run when 'success' then succeed @@ -516,6 +531,9 @@ module Ci when 'canceled' then cancel when 'skipped' then skip when 'manual' then block + else + raise HasStatus::UnknownStatusError, + "Unknown status `#{latest_builds_status}`" end end end diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 57edd6a4956..8c9aacca8de 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -219,10 +219,8 @@ module Ci cache_attributes(values) - if persist_cached_data? - self.assign_attributes(values) - self.save if self.changed? - end + # We save data without validation, it will always change due to `contacted_at` + self.update_columns(values) if persist_cached_data? end def pick_build!(build) diff --git a/app/models/ci/stage.rb b/app/models/ci/stage.rb index 5a1eeb966aa..ea07f37e6c1 100644 --- a/app/models/ci/stage.rb +++ b/app/models/ci/stage.rb @@ -68,16 +68,44 @@ module Ci def update_status retry_optimistic_lock(self) do case statuses.latest.status + when 'created' then nil when 'pending' then enqueue when 'running' then run when 'success' then succeed when 'failed' then drop when 'canceled' then cancel when 'manual' then block - when 'skipped' then skip - else skip + when 'skipped', nil then skip + else + raise HasStatus::UnknownStatusError, + "Unknown status `#{statuses.latest.status}`" end end end + + def groups + @groups ||= Ci::Group.fabricate(self) + end + + def has_warnings? + number_of_warnings.positive? + end + + def number_of_warnings + BatchLoader.for(id).batch(default_value: 0) do |stage_ids, loader| + ::Ci::Build.where(stage_id: stage_ids) + .latest + .failed_but_allowed + .group(:stage_id) + .count + .each { |id, amount| loader.call(id, amount) } + end + end + + def detailed_status(current_user) + Gitlab::Ci::Status::Stage::Factory + .new(self, current_user) + .fabricate! + end end end diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb index 13246a774e3..095897b08e3 100644 --- a/app/models/concerns/avatarable.rb +++ b/app/models/concerns/avatarable.rb @@ -4,11 +4,14 @@ module Avatarable included do prepend ShadowMethods include ObjectStorage::BackgroundMove + include Gitlab::Utils::StrongMemoize validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? } validates :avatar, file_size: { maximum: 200.kilobytes.to_i } mount_uploader :avatar, AvatarUploader + + after_initialize :add_avatar_to_batch end module ShadowMethods @@ -18,6 +21,17 @@ module Avatarable avatar_path(only_path: args.fetch(:only_path, true)) || super end + + def retrieve_upload(identifier, paths) + upload = retrieve_upload_from_batch(identifier) + + # This fallback is needed when deleting an upload, because we may have + # already been removed from the DB. We have to check an explicit `#nil?` + # because it's a BatchLoader instance. + upload = super if upload.nil? + + upload + end end def avatar_type @@ -52,4 +66,37 @@ module Avatarable url_base + avatar.local_url end + + # Path that is persisted in the tracking Upload model. Used to fetch the + # upload from the model. + def upload_paths(identifier) + avatar_mounter.blank_uploader.store_dirs.map { |store, path| File.join(path, identifier) } + end + + private + + def retrieve_upload_from_batch(identifier) + BatchLoader.for(identifier: identifier, model: self).batch(key: self.class) do |upload_params, loader, args| + model_class = args[:key] + paths = upload_params.flat_map do |params| + params[:model].upload_paths(params[:identifier]) + end + + Upload.where(uploader: AvatarUploader, path: paths).find_each do |upload| + model = model_class.instantiate('id' => upload.model_id) + + loader.call({ model: model, identifier: File.basename(upload.path) }, upload) + end + end + end + + def add_avatar_to_batch + return unless avatar_mounter + + avatar_mounter.read_identifiers.each { |identifier| retrieve_upload_from_batch(identifier) } + end + + def avatar_mounter + strong_memoize(:avatar_mounter) { _mounter(:avatar) } + end end diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb index 7c3ed96bc28..72c236a0fc7 100644 --- a/app/models/concerns/has_status.rb +++ b/app/models/concerns/has_status.rb @@ -11,6 +11,8 @@ module HasStatus STATUSES_ENUM = { created: 0, pending: 1, running: 2, success: 3, failed: 4, canceled: 5, skipped: 6, manual: 7 }.freeze + UnknownStatusError = Class.new(StandardError) + class_methods do def status_sql scope_relevant = respond_to?(:exclude_ignored) ? exclude_ignored : all diff --git a/app/models/concerns/with_uploads.rb b/app/models/concerns/with_uploads.rb index e7cfffb775b..4245d083a49 100644 --- a/app/models/concerns/with_uploads.rb +++ b/app/models/concerns/with_uploads.rb @@ -36,4 +36,8 @@ module WithUploads upload.destroy end end + + def retrieve_upload(_identifier, paths) + uploads.find_by(path: paths) + end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 4c1628d2bdb..535a2c362f2 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1125,21 +1125,21 @@ class MergeRequest < ActiveRecord::Base project.merge_requests.merged.where(author_id: author_id).empty? end - def allow_maintainer_to_push - maintainer_push_possible? && super + def allow_collaboration + collaborative_push_possible? && super end - alias_method :allow_maintainer_to_push?, :allow_maintainer_to_push + alias_method :allow_collaboration?, :allow_collaboration - def maintainer_push_possible? + def collaborative_push_possible? source_project.present? && for_fork? && target_project.visibility_level > Gitlab::VisibilityLevel::PRIVATE && source_project.visibility_level > Gitlab::VisibilityLevel::PRIVATE && !ProtectedBranch.protected?(source_project, source_branch) end - def can_allow_maintainer_to_push?(user) - maintainer_push_possible? && + def can_allow_collaboration?(user) + collaborative_push_possible? && Ability.allowed?(user, :push_code, source_project) end diff --git a/app/models/note.rb b/app/models/note.rb index 02f7a9b1e4f..41c04ae0571 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -435,6 +435,10 @@ class Note < ActiveRecord::Base super.merge(noteable: noteable) end + def retrieve_upload(_identifier, paths) + Upload.find_by(model: self, path: paths) + end + private def keep_around_commit diff --git a/app/models/personal_snippet.rb b/app/models/personal_snippet.rb index 82c1c4de3a0..355624fd552 100644 --- a/app/models/personal_snippet.rb +++ b/app/models/personal_snippet.rb @@ -1,2 +1,3 @@ class PersonalSnippet < Snippet + include WithUploads end diff --git a/app/models/project.rb b/app/models/project.rb index a094dbcb747..562198e2369 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -228,6 +228,7 @@ class Project < ActiveRecord::Base has_many :commit_statuses has_many :pipelines, class_name: 'Ci::Pipeline', inverse_of: :project + has_many :stages, class_name: 'Ci::Stage', inverse_of: :project # Ci::Build objects store data on the file system such as artifact files and # build traces. Currently there's no efficient way of removing this data in @@ -1425,8 +1426,14 @@ class Project < ActiveRecord::Base Ci::Runner.from("(#{union.to_sql}) ci_runners") end + def active_runners + strong_memoize(:active_runners) do + all_runners.active + end + end + def any_runners?(&block) - all_runners.active.any?(&block) + active_runners.any?(&block) end def valid_runners_token?(token) @@ -1968,18 +1975,18 @@ class Project < ActiveRecord::Base .limit(1) .select(1) source_of_merge_requests.opened - .where(allow_maintainer_to_push: true) + .where(allow_collaboration: true) .where('EXISTS (?)', developer_access_exists) end - def branch_allows_maintainer_push?(user, branch_name) + def branch_allows_collaboration?(user, branch_name) return false unless user cache_key = "user:#{user.id}:#{branch_name}:branch_allows_push" - memoized_results = strong_memoize(:branch_allows_maintainer_push) do + memoized_results = strong_memoize(:branch_allows_collaboration) do Hash.new do |result, cache_key| - result[cache_key] = fetch_branch_allows_maintainer_push?(user, branch_name) + result[cache_key] = fetch_branch_allows_collaboration?(user, branch_name) end end @@ -2121,18 +2128,18 @@ class Project < ActiveRecord::Base raise ex end - def fetch_branch_allows_maintainer_push?(user, branch_name) + def fetch_branch_allows_collaboration?(user, branch_name) check_access = -> do next false if empty_repo? merge_request = source_of_merge_requests.opened - .where(allow_maintainer_to_push: true) + .where(allow_collaboration: true) .find_by(source_branch: branch_name) merge_request&.can_be_merged_by?(user) end if RequestStore.active? - RequestStore.fetch("project-#{id}:branch-#{branch_name}:user-#{user.id}:branch_allows_maintainer_push") do + RequestStore.fetch("project-#{id}:branch-#{branch_name}:user-#{user.id}:branch_allows_collaboration") do check_access.call end else diff --git a/app/models/repository.rb b/app/models/repository.rb index 82cf47ba04e..0784891d1bf 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -270,6 +270,16 @@ class Repository end end + def archive_metadata(ref, storage_path, format = "tar.gz", append_sha:) + raw_repository.archive_metadata( + ref, + storage_path, + project.path, + format, + append_sha: append_sha + ) + end + def expire_tags_cache expire_method_caches(%i(tag_names tag_count)) @tags = nil diff --git a/app/models/term_agreement.rb b/app/models/term_agreement.rb index 8458a231bbd..c317bd0c90b 100644 --- a/app/models/term_agreement.rb +++ b/app/models/term_agreement.rb @@ -2,5 +2,7 @@ class TermAgreement < ActiveRecord::Base belongs_to :term, class_name: 'ApplicationSetting::Term' belongs_to :user + scope :accepted, -> { where(accepted: true) } + validates :user, :term, presence: true end diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb index 8b65758f3e8..1c0cc7425ec 100644 --- a/app/policies/ci/build_policy.rb +++ b/app/policies/ci/build_policy.rb @@ -14,8 +14,8 @@ module Ci @subject.triggered_by?(@user) end - condition(:branch_allows_maintainer_push) do - @subject.project.branch_allows_maintainer_push?(@user, @subject.ref) + condition(:branch_allows_collaboration) do + @subject.project.branch_allows_collaboration?(@user, @subject.ref) end rule { protected_ref }.policy do @@ -25,7 +25,7 @@ module Ci rule { can?(:admin_build) | (can?(:update_build) & owner_of_job) }.enable :erase_build - rule { can?(:public_access) & branch_allows_maintainer_push }.policy do + rule { can?(:public_access) & branch_allows_collaboration }.policy do enable :update_build enable :update_commit_status end diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb index 540e4235299..b81329d0625 100644 --- a/app/policies/ci/pipeline_policy.rb +++ b/app/policies/ci/pipeline_policy.rb @@ -4,13 +4,13 @@ module Ci condition(:protected_ref) { ref_protected?(@user, @subject.project, @subject.tag?, @subject.ref) } - condition(:branch_allows_maintainer_push) do - @subject.project.branch_allows_maintainer_push?(@user, @subject.ref) + condition(:branch_allows_collaboration) do + @subject.project.branch_allows_collaboration?(@user, @subject.ref) end rule { protected_ref }.prevent :update_pipeline - rule { can?(:public_access) & branch_allows_maintainer_push }.policy do + rule { can?(:public_access) & branch_allows_collaboration }.policy do enable :update_pipeline end diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index 141070aef45..8260c6c7b84 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -13,7 +13,7 @@ class MergeRequestWidgetEntity < IssuableEntity expose :squash expose :target_branch expose :target_project_id - expose :allow_maintainer_to_push + expose :allow_collaboration expose :should_be_rebased?, as: :should_be_rebased expose :ff_only_enabled do |merge_request| diff --git a/app/serializers/pipeline_details_entity.rb b/app/serializers/pipeline_details_entity.rb index 130968a44c1..8ba9cac53c4 100644 --- a/app/serializers/pipeline_details_entity.rb +++ b/app/serializers/pipeline_details_entity.rb @@ -1,6 +1,6 @@ class PipelineDetailsEntity < PipelineEntity expose :details do - expose :legacy_stages, as: :stages, using: StageEntity + expose :ordered_stages, as: :stages, using: StageEntity expose :artifacts, using: BuildArtifactEntity expose :manual_actions, using: BuildActionEntity end diff --git a/app/serializers/pipeline_serializer.rb b/app/serializers/pipeline_serializer.rb index 7181f8a6b04..17a022539bb 100644 --- a/app/serializers/pipeline_serializer.rb +++ b/app/serializers/pipeline_serializer.rb @@ -1,14 +1,11 @@ class PipelineSerializer < BaseSerializer include WithPagination - - InvalidResourceError = Class.new(StandardError) - entity PipelineDetailsEntity def represent(resource, opts = {}) if resource.is_a?(ActiveRecord::Relation) - resource = resource.preload([ + :stages, :retryable_builds, :cancelable_statuses, :trigger_requests, @@ -20,10 +17,14 @@ class PipelineSerializer < BaseSerializer end if paginated? - super(@paginator.paginate(resource), opts) - else - super(resource, opts) + resource = paginator.paginate(resource) end + + if opts.delete(:preload) + resource = Gitlab::Ci::Pipeline::Preloader.preload!(resource) + end + + super(resource, opts) end def represent_status(resource) @@ -36,7 +37,7 @@ class PipelineSerializer < BaseSerializer def represent_stages(resource) return {} unless resource.present? - data = represent(resource, { only: [{ details: [:stages] }] }) + data = represent(resource, { only: [{ details: [:stages] }], preload: true }) data.dig(:details, :stages) || [] end end diff --git a/app/services/application_settings/update_service.rb b/app/services/application_settings/update_service.rb index e70445cfb67..7bcb8f49d0d 100644 --- a/app/services/application_settings/update_service.rb +++ b/app/services/application_settings/update_service.rb @@ -1,5 +1,7 @@ module ApplicationSettings class UpdateService < ApplicationSettings::BaseService + attr_reader :params, :application_setting + def execute update_terms(@params.delete(:terms)) diff --git a/app/services/applications/create_service.rb b/app/services/applications/create_service.rb index 35d45f25a71..e67af929954 100644 --- a/app/services/applications/create_service.rb +++ b/app/services/applications/create_service.rb @@ -2,8 +2,7 @@ module Applications class CreateService def initialize(current_user, params) @current_user = current_user - @params = params - @ip_address = @params.delete(:ip_address) + @params = params.except(:ip_address) end def execute(request = nil) diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 231ab76fde4..4c420b38258 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -38,8 +38,8 @@ module MergeRequests def filter_params(merge_request) super - unless merge_request.can_allow_maintainer_to_push?(current_user) - params.delete(:allow_maintainer_to_push) + unless merge_request.can_allow_collaboration?(current_user) + params.delete(:allow_collaboration) end end diff --git a/app/services/test_hooks/project_service.rb b/app/services/test_hooks/project_service.rb index 01d5d774cd5..65183e84cce 100644 --- a/app/services/test_hooks/project_service.rb +++ b/app/services/test_hooks/project_service.rb @@ -1,11 +1,13 @@ module TestHooks class ProjectService < TestHooks::BaseService - private + attr_writer :project def project @project ||= hook.project end + private + def push_events_data throw(:validation_error, 'Ensure the project has at least one commit.') if project.empty_repo? diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index 3bb2e1ea63a..5aa1bc7227c 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -33,7 +33,7 @@ module ObjectStorage unless current_upload_satisfies?(paths, model) # the upload we already have isn't right, find the correct one - self.upload = uploads.find_by(model: model, path: paths) + self.upload = model&.retrieve_upload(identifier, paths) end super @@ -46,7 +46,7 @@ module ObjectStorage end def upload=(upload) - return unless upload + return if upload.nil? self.object_store = upload.store super diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml index 0a22a142858..ccba1c461fc 100644 --- a/app/views/admin/projects/show.html.haml +++ b/app/views/admin/projects/show.html.haml @@ -116,8 +116,8 @@ .card-body = form_for @project, url: transfer_admin_project_path(@project), method: :put do |f| .form-group.row - = f.label :new_namespace_id, "Namespace", class: 'col-form-label col-sm-2' - .col-sm-10 + = f.label :new_namespace_id, "Namespace", class: 'col-form-label col-sm-3' + .col-sm-9 .dropdown = dropdown_toggle('Search for Namespace', { toggle: 'dropdown', field_name: 'new_namespace_id' }, { toggle_class: 'js-namespace-select large' }) .dropdown-menu.dropdown-select @@ -127,7 +127,7 @@ = dropdown_loading .form-group.row - .offset-sm-2.col-sm-10 + .offset-sm-3.col-sm-9 = f.submit 'Transfer', class: 'btn btn-primary' .card.repository-check diff --git a/app/views/admin/runners/index.html.haml b/app/views/admin/runners/index.html.haml index f38aeb151df..8dfd176f1b7 100644 --- a/app/views/admin/runners/index.html.haml +++ b/app/views/admin/runners/index.html.haml @@ -67,7 +67,7 @@ %th Projects %th Jobs %th Tags - %th= link_to 'Last contact', admin_runners_path(params.slice(:search).merge(sort: 'contacted_asc')) + %th= link_to 'Last contact', admin_runners_path(safe_params.slice(:search).merge(sort: 'contacted_asc')) %th - @runners.each do |runner| diff --git a/app/views/groups/_activities.html.haml b/app/views/groups/_activities.html.haml index fd6e7111f38..577c63503a8 100644 --- a/app/views/groups/_activities.html.haml +++ b/app/views/groups/_activities.html.haml @@ -1,4 +1,4 @@ -.nav-block +.nav-block.activities .controls = link_to group_path(@group, rss_url_options), class: 'btn rss-btn has-tooltip' , title: 'Subscribe' do %i.fa.fa-rss diff --git a/app/views/help/_shortcuts.html.haml b/app/views/help/_shortcuts.html.haml index 29db29235c1..c23fe0b5c49 100644 --- a/app/views/help/_shortcuts.html.haml +++ b/app/views/help/_shortcuts.html.haml @@ -18,71 +18,71 @@ %th Global Shortcuts %tr %td.shortcut - .key s + %kbd s %td Focus Search %tr %td.shortcut - .key f + %kbd f %td Focus Filter - if performance_bar_enabled? %tr %td.shortcut - .key p b + %kbd p b %td Show/hide the Performance Bar %tr %td.shortcut - .key ? + %kbd ? %td Show/hide this dialog %tr %td.shortcut - if browser.platform.mac? - .key ⌘ shift p + %kbd ⌘ shift p - else - .key ctrl shift p + %kbd ctrl shift p %td Toggle Markdown preview %tr %td.shortcut - .key + %kbd %i.fa.fa-arrow-up %td Edit last comment (when focused on an empty textarea) %tr %td.shortcut - .key shift t + %kbd shift t %td Go to todos %tr %td.shortcut - .key shift a + %kbd shift a %td Go to the activity feed %tr %td.shortcut - .key shift p + %kbd shift p %td Go to projects %tr %td.shortcut - .key shift i + %kbd shift i %td Go to issues %tr %td.shortcut - .key shift m + %kbd shift m %td Go to merge requests %tr %td.shortcut - .key shift g + %kbd shift g %td Go to groups %tr %td.shortcut - .key shift l + %kbd shift l %td Go to milestones %tr %td.shortcut - .key shift s + %kbd shift s %td Go to snippets %tbody @@ -91,21 +91,21 @@ %th Finding Project File %tr %td.shortcut - .key + %kbd %i.fa.fa-arrow-up %td Move selection up %tr %td.shortcut - .key + %kbd %i.fa.fa-arrow-down %td Move selection down %tr %td.shortcut - .key enter + %kbd enter %td Open Selection %tr %td.shortcut - .key esc + %kbd esc %td Go back .col-lg-4 %table.shortcut-mappings @@ -115,95 +115,95 @@ %th Project %tr %td.shortcut - .key g - .key p + %kbd g + %kbd p %td Go to the project's overview page %tr %td.shortcut - .key g - .key v + %kbd g + %kbd v %td Go to the project's activity feed %tr %td.shortcut - .key g - .key f + %kbd g + %kbd f %td Go to files %tr %td.shortcut - .key g - .key c + %kbd g + %kbd c %td Go to commits %tr %td.shortcut - .key g - .key j + %kbd g + %kbd j %td Go to jobs %tr %td.shortcut - .key g - .key n + %kbd g + %kbd n %td Go to network graph %tr %td.shortcut - .key g - .key d + %kbd g + %kbd d %td Go to repository charts %tr %td.shortcut - .key g - .key i + %kbd g + %kbd i %td Go to issues %tr %td.shortcut - .key g - .key b + %kbd g + %kbd b %td Go to issue boards %tr %td.shortcut - .key g - .key m + %kbd g + %kbd m %td Go to merge requests %tr %td.shortcut - .key g - .key e + %kbd g + %kbd e %td Go to environments %tr %td.shortcut - .key g - .key k + %kbd g + %kbd k %td Go to kubernetes %tr %td.shortcut - .key g - .key s + %kbd g + %kbd s %td Go to snippets %tr %td.shortcut - .key g - .key w + %kbd g + %kbd w %td Go to wiki %tr %td.shortcut - .key t + %kbd t %td Go to finding file %tr %td.shortcut - .key i + %kbd i %td New issue %tbody @@ -212,17 +212,17 @@ %th Project Files browsing %tr %td.shortcut - .key + %kbd %i.fa.fa-arrow-up %td Move selection up %tr %td.shortcut - .key + %kbd %i.fa.fa-arrow-down %td Move selection down %tr %td.shortcut - .key enter + %kbd enter %td Open Selection %tbody %tr @@ -230,7 +230,7 @@ %th Project File %tr %td.shortcut - .key y + %kbd y %td Go to file permalink %tbody %tr @@ -239,115 +239,115 @@ %tr %td.shortcut - if browser.platform.mac? - .key ⌘ p + %kbd ⌘ p - else - .key ctrl p + %kbd ctrl p %td Go to file .col-lg-4 %table.shortcut-mappings - %tbody.hidden-shortcut.network{ style: 'display:none' } + %tbody.hidden-shortcut{ style: 'display:none' } %tr %th %th Network Graph %tr %td.shortcut - .key + %kbd %i.fa.fa-arrow-left \/ - .key h + %kbd h %td Scroll left %tr %td.shortcut - .key + %kbd %i.fa.fa-arrow-right \/ - .key l + %kbd l %td Scroll right %tr %td.shortcut - .key + %kbd %i.fa.fa-arrow-up \/ - .key k + %kbd k %td Scroll up %tr %td.shortcut - .key + %kbd %i.fa.fa-arrow-down \/ - .key j + %kbd j %td Scroll down %tr %td.shortcut - .key + %kbd shift %i.fa.fa-arrow-up \/ - .key + %kbd shift k %td Scroll to top %tr %td.shortcut - .key + %kbd shift %i.fa.fa-arrow-down \/ - .key + %kbd shift j %td Scroll to bottom - %tbody.hidden-shortcut.issues{ style: 'display:none' } + %tbody.hidden-shortcut{ style: 'display:none' } %tr %th %th Issues %tr %td.shortcut - .key a + %kbd a %td Change assignee %tr %td.shortcut - .key m + %kbd m %td Change milestone %tr %td.shortcut - .key r + %kbd r %td Reply (quoting selected text) %tr %td.shortcut - .key e + %kbd e %td Edit issue %tr %td.shortcut - .key l + %kbd l %td Change Label - %tbody.hidden-shortcut.merge_requests{ style: 'display:none' } + %tbody.hidden-shortcut{ style: 'display:none' } %tr %th %th Merge Requests %tr %td.shortcut - .key a + %kbd a %td Change assignee %tr %td.shortcut - .key m + %kbd m %td Change milestone %tr %td.shortcut - .key r + %kbd r %td Reply (quoting selected text) %tr %td.shortcut - .key e + %kbd e %td Edit merge request %tr %td.shortcut - .key l + %kbd l %td Change Label - %tbody.hidden-shortcut.wiki{ style: 'display:none' } + %tbody.hidden-shortcut{ style: 'display:none' } %tr %th %th Wiki pages %tr %td.shortcut - .key e + %kbd e %td Edit wiki page diff --git a/app/views/kaminari/gitlab/_first_page.html.haml b/app/views/kaminari/gitlab/_first_page.html.haml index 369165da02a..3b7d4a1c578 100644 --- a/app/views/kaminari/gitlab/_first_page.html.haml +++ b/app/views/kaminari/gitlab/_first_page.html.haml @@ -5,5 +5,5 @@ -# total_pages: total number of pages -# per_page: number of items to fetch per page -# remote: data-remote -%li.first.page-item +%li.page-item.js-first-button = link_to_unless current_page.first?, raw(t 'views.pagination.first'), url, remote: remote, class: 'page-link' diff --git a/app/views/kaminari/gitlab/_gap.html.haml b/app/views/kaminari/gitlab/_gap.html.haml index 6eec30212d1..849f92fdc95 100644 --- a/app/views/kaminari/gitlab/_gap.html.haml +++ b/app/views/kaminari/gitlab/_gap.html.haml @@ -4,5 +4,5 @@ -# total_pages: total number of pages -# per_page: number of items to fetch per page -# remote: data-remote -%li.page-item.disabled +%li.page-item.disabled.d-none.d-md-block = link_to raw(t 'views.pagination.truncate'), '#', class: 'page-link' diff --git a/app/views/kaminari/gitlab/_last_page.html.haml b/app/views/kaminari/gitlab/_last_page.html.haml index 8b49db58281..7836e17f877 100644 --- a/app/views/kaminari/gitlab/_last_page.html.haml +++ b/app/views/kaminari/gitlab/_last_page.html.haml @@ -5,5 +5,5 @@ -# total_pages: total number of pages -# per_page: number of items to fetch per page -# remote: data-remote -%li.last.page-item +%li.page-item.js-last-button = link_to_unless current_page.last?, raw(t 'views.pagination.last'), url, {remote: remote, class: 'page-link'} diff --git a/app/views/kaminari/gitlab/_next_page.html.haml b/app/views/kaminari/gitlab/_next_page.html.haml index 05f151555ad..a7fa1a21a6c 100644 --- a/app/views/kaminari/gitlab/_next_page.html.haml +++ b/app/views/kaminari/gitlab/_next_page.html.haml @@ -8,5 +8,5 @@ - page_url = current_page.last? ? '#' : url -%li.page-item{ class: ('disabled' if current_page.last?) } +%li.page-item.js-next-button{ class: ('disabled' if current_page.last?) } = link_to raw(t 'views.pagination.next'), page_url, rel: 'next', remote: remote, class: 'page-link' diff --git a/app/views/kaminari/gitlab/_page.html.haml b/app/views/kaminari/gitlab/_page.html.haml index 8a40e13a537..d0dc1784540 100644 --- a/app/views/kaminari/gitlab/_page.html.haml +++ b/app/views/kaminari/gitlab/_page.html.haml @@ -6,5 +6,5 @@ -# total_pages: total number of pages -# per_page: number of items to fetch per page -# remote: data-remote -%li.page-item.js-pagination-page{ class: [active_when(page.current?), ('sibling' if page.next? || page.prev?)] } +%li.page-item.js-pagination-page{ class: [active_when(page.current?), ('sibling' if page.next? || page.prev?), ('d-none d-md-block' if !page.current?) ] } = link_to page, url, { remote: remote, rel: page.next? ? 'next' : page.prev? ? 'prev' : nil, class: 'page-link' } diff --git a/app/views/kaminari/gitlab/_paginator.html.haml b/app/views/kaminari/gitlab/_paginator.html.haml index a6435deb4bf..ac9e274dbc7 100644 --- a/app/views/kaminari/gitlab/_paginator.html.haml +++ b/app/views/kaminari/gitlab/_paginator.html.haml @@ -6,7 +6,7 @@ -# remote: data-remote -# paginator: the paginator that renders the pagination tags inside = paginator.render do - .gl-pagination + .gl-pagination.prepend-top-default %ul.pagination.justify-content-center - unless current_page.first? = first_page_tag unless total_pages < 5 # As kaminari will always show the first 5 pages diff --git a/app/views/kaminari/gitlab/_prev_page.html.haml b/app/views/kaminari/gitlab/_prev_page.html.haml index f4a11a449b7..12b0e106a62 100644 --- a/app/views/kaminari/gitlab/_prev_page.html.haml +++ b/app/views/kaminari/gitlab/_prev_page.html.haml @@ -8,5 +8,5 @@ - page_url = current_page.first? ? '#' : url -%li.page-item{ class: ('disabled' if current_page.first?) } +%li.page-item.js-previous-button{ class: ('disabled' if current_page.first?) } = link_to raw(t 'views.pagination.previous'), page_url, rel: 'prev', remote: remote, class: 'page-link' diff --git a/app/views/kaminari/gitlab/_without_count.html.haml b/app/views/kaminari/gitlab/_without_count.html.haml index 1425a809052..f780400ebcb 100644 --- a/app/views/kaminari/gitlab/_without_count.html.haml +++ b/app/views/kaminari/gitlab/_without_count.html.haml @@ -1,5 +1,5 @@ -.gl-pagination - %ul.pagination.clearfix +.gl-pagination.prepend-top-default + %ul.pagination.justify-content-center - if previous_path %li.page-item.prev = link_to(t('views.pagination.previous'), previous_path, rel: 'prev', class: 'page-link') diff --git a/app/views/projects/new.html.haml b/app/views/projects/new.html.haml index 35a09f06bfa..5bb1bfb7059 100644 --- a/app/views/projects/new.html.haml +++ b/app/views/projects/new.html.haml @@ -29,16 +29,16 @@ .col-lg-9.js-toggle-container %ul.nav.nav-tabs.nav-links.gitlab-tabs{ role: 'tablist' } - %li{ class: active_when(active_tab == 'blank'), role: 'presentation' } - %a{ href: '#blank-project-pane', id: 'blank-project-tab', data: { toggle: 'tab' }, role: 'tab' } + %li.nav-item{ role: 'presentation' } + %a.nav-link.active{ href: '#blank-project-pane', id: 'blank-project-tab', data: { toggle: 'tab' }, role: 'tab' } %span.d-none.d-sm-block Blank project %span.d-block.d-sm-none Blank - %li{ class: active_when(active_tab == 'template'), role: 'presentation' } - %a{ href: '#create-from-template-pane', id: 'create-from-template-tab', data: { toggle: 'tab' }, role: 'tab' } + %li.nav-item{ role: 'presentation' } + %a.nav-link{ href: '#create-from-template-pane', id: 'create-from-template-tab', data: { toggle: 'tab' }, role: 'tab' } %span.d-none.d-sm-block Create from template %span.d-block.d-sm-none Template - %li{ class: active_when(active_tab == 'import'), role: 'presentation' } - %a{ href: '#import-project-pane', id: 'import-project-tab', data: { toggle: 'tab' }, role: 'tab' } + %li.nav-item{ role: 'presentation' } + %a.nav-link{ href: '#import-project-pane', id: 'import-project-tab', data: { toggle: 'tab' }, role: 'tab' } %span.d-none.d-sm-block Import project %span.d-block.d-sm-none Import diff --git a/app/views/projects/pages/_destroy.haml b/app/views/projects/pages/_destroy.haml index 4ada19a1368..9b77c4e3494 100644 --- a/app/views/projects/pages/_destroy.haml +++ b/app/views/projects/pages/_destroy.haml @@ -5,7 +5,7 @@ .errors-holder .card-body %p - Removing the pages will prevent from exposing them to outside world. + Removing pages will prevent them from being exposed to the outside world. .form-actions = link_to 'Remove pages', project_pages_path(@project), data: { confirm: 'Are you sure?'}, method: :delete, class: "btn btn-remove" - else diff --git a/app/views/shared/issuable/form/_contribution.html.haml b/app/views/shared/issuable/form/_contribution.html.haml index b34549240e0..519b5fae846 100644 --- a/app/views/shared/issuable/form/_contribution.html.haml +++ b/app/views/shared/issuable/form/_contribution.html.haml @@ -12,9 +12,9 @@ = _('Contribution') .col-sm-10 .form-check - = form.check_box :allow_maintainer_to_push, disabled: !issuable.can_allow_maintainer_to_push?(current_user), class: 'form-check-input' - = form.label :allow_maintainer_to_push, class: 'form-check-label' do - = _('Allow edits from maintainers.') - = link_to 'About this feature', help_page_path('user/project/merge_requests/maintainer_access') + = form.check_box :allow_collaboration, disabled: !issuable.can_allow_collaboration?(current_user), class: 'form-check-input' + = form.label :allow_collaboration, class: 'form-check-label' do + = _('Allow commits from members who can merge to the target branch.') + = link_to 'About this feature', help_page_path('user/project/merge_requests/allow_collaboration') .form-text.text-muted - = allow_maintainer_push_unavailable_reason(issuable) + = allow_collaboration_unavailable_reason(issuable) diff --git a/app/views/shared/projects/_edit_information.html.haml b/app/views/shared/projects/_edit_information.html.haml index ec9dc8f62c2..9230e045a81 100644 --- a/app/views/shared/projects/_edit_information.html.haml +++ b/app/views/shared/projects/_edit_information.html.haml @@ -1,6 +1,6 @@ - unless can?(current_user, :push_code, @project) .inline.prepend-left-10 - - if @project.branch_allows_maintainer_push?(current_user, selected_branch) + - if @project.branch_allows_collaboration?(current_user, selected_branch) = commit_in_single_accessible_branch - else = commit_in_fork_help diff --git a/app/views/users/terms/index.html.haml b/app/views/users/terms/index.html.haml index b9f25a71170..33cddf63952 100644 --- a/app/views/users/terms/index.html.haml +++ b/app/views/users/terms/index.html.haml @@ -7,6 +7,10 @@ .float-right = button_to accept_term_path(@term, redirect_params), class: 'btn btn-success prepend-left-8' do = _('Accept terms') + - else + .pull-right + = link_to root_path, class: 'btn btn-success prepend-left-8' do + = _('Continue') - if can?(current_user, :decline_terms, @term) .float-right = button_to decline_term_path(@term, redirect_params), class: 'btn btn-default prepend-left-8' do diff --git a/changelogs/unreleased/25045-add-variables-to-post-pipeline-api.yml b/changelogs/unreleased/25045-add-variables-to-post-pipeline-api.yml new file mode 100644 index 00000000000..1e648b75248 --- /dev/null +++ b/changelogs/unreleased/25045-add-variables-to-post-pipeline-api.yml @@ -0,0 +1,5 @@ +--- +title: Add variables to POST api/v4/projects/:id/pipeline +merge_request: 19124 +author: Jacopo Beschi @jacopo-beschi +type: added diff --git a/changelogs/unreleased/42751-rename-mr-maintainer-push.yml b/changelogs/unreleased/42751-rename-mr-maintainer-push.yml new file mode 100644 index 00000000000..aa29f6ed4b7 --- /dev/null +++ b/changelogs/unreleased/42751-rename-mr-maintainer-push.yml @@ -0,0 +1,5 @@ +--- +title: Rephrasing Merge Request's 'allow edits from maintainer' functionality +merge_request: 19061 +author: +type: deprecated diff --git a/changelogs/unreleased/45702-fix-hashed-storage-repository-archive.yml b/changelogs/unreleased/45702-fix-hashed-storage-repository-archive.yml new file mode 100644 index 00000000000..0f85ced06a9 --- /dev/null +++ b/changelogs/unreleased/45702-fix-hashed-storage-repository-archive.yml @@ -0,0 +1,5 @@ +--- +title: Fix repository archive generation when hashed storage is enabled +merge_request: 19441 +author: +type: fixed diff --git a/changelogs/unreleased/46585-gdpr-terms-acceptance.yml b/changelogs/unreleased/46585-gdpr-terms-acceptance.yml new file mode 100644 index 00000000000..84853846b0e --- /dev/null +++ b/changelogs/unreleased/46585-gdpr-terms-acceptance.yml @@ -0,0 +1,6 @@ +--- +title: Add flash notice if user has already accepted terms and allow users to continue + to root path +merge_request: 19156 +author: +type: changed diff --git a/changelogs/unreleased/fix-avatars-n-plus-one.yml b/changelogs/unreleased/fix-avatars-n-plus-one.yml new file mode 100644 index 00000000000..c5b42071f2b --- /dev/null +++ b/changelogs/unreleased/fix-avatars-n-plus-one.yml @@ -0,0 +1,5 @@ +--- +title: Fix an N+1 when loading user avatars +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/optimise-runner-update-cached-info.yml b/changelogs/unreleased/optimise-runner-update-cached-info.yml new file mode 100644 index 00000000000..15fb9bcdf41 --- /dev/null +++ b/changelogs/unreleased/optimise-runner-update-cached-info.yml @@ -0,0 +1,5 @@ +--- +title: Update runner cached informations without performing validations +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/rails5-fix-47368.yml b/changelogs/unreleased/rails5-fix-47368.yml new file mode 100644 index 00000000000..81bb1adabff --- /dev/null +++ b/changelogs/unreleased/rails5-fix-47368.yml @@ -0,0 +1,6 @@ +--- +title: 'Rails 5 fix unknown keywords: changes, key_id, project, gl_repository, action, + secret_token, protocol' +merge_request: 19466 +author: Jasper Maes +type: fixed diff --git a/changelogs/unreleased/rd-44364-deprecate-support-for-dsa-keys.yml b/changelogs/unreleased/rd-44364-deprecate-support-for-dsa-keys.yml new file mode 100644 index 00000000000..1a52ffaaf79 --- /dev/null +++ b/changelogs/unreleased/rd-44364-deprecate-support-for-dsa-keys.yml @@ -0,0 +1,5 @@ +--- +title: Add migration to disable the usage of DSA keys +merge_request: 19299 +author: +type: other diff --git a/changelogs/unreleased/sh-fix-events-nplus-one.yml b/changelogs/unreleased/sh-fix-events-nplus-one.yml new file mode 100644 index 00000000000..e5a974bef30 --- /dev/null +++ b/changelogs/unreleased/sh-fix-events-nplus-one.yml @@ -0,0 +1,5 @@ +--- +title: Eliminate N+1 queries with authors and push_data_payload in Events API +merge_request: +author: +type: performance diff --git a/db/migrate/20180523042841_rename_merge_requests_allow_maintainer_to_push.rb b/db/migrate/20180523042841_rename_merge_requests_allow_maintainer_to_push.rb new file mode 100644 index 00000000000..975bdfe70f4 --- /dev/null +++ b/db/migrate/20180523042841_rename_merge_requests_allow_maintainer_to_push.rb @@ -0,0 +1,15 @@ +class RenameMergeRequestsAllowMaintainerToPush < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + rename_column_concurrently :merge_requests, :allow_maintainer_to_push, :allow_collaboration + end + + def down + cleanup_concurrent_column_rename :merge_requests, :allow_collaboration, :allow_maintainer_to_push + end +end diff --git a/db/migrate/20180530135500_add_index_to_stages_position.rb b/db/migrate/20180530135500_add_index_to_stages_position.rb new file mode 100644 index 00000000000..61150f33a25 --- /dev/null +++ b/db/migrate/20180530135500_add_index_to_stages_position.rb @@ -0,0 +1,15 @@ +class AddIndexToStagesPosition < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :ci_stages, [:pipeline_id, :position] + end + + def down + remove_concurrent_index :ci_stages, [:pipeline_id, :position] + end +end diff --git a/db/migrate/20180531220618_change_default_value_for_dsa_key_restriction.rb b/db/migrate/20180531220618_change_default_value_for_dsa_key_restriction.rb new file mode 100644 index 00000000000..d0dcacc5b66 --- /dev/null +++ b/db/migrate/20180531220618_change_default_value_for_dsa_key_restriction.rb @@ -0,0 +1,16 @@ +class ChangeDefaultValueForDsaKeyRestriction < ActiveRecord::Migration + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + def up + change_column :application_settings, :dsa_key_restriction, :integer, null: false, + default: -1 + + execute("UPDATE application_settings SET dsa_key_restriction = -1") + end + + def down + change_column :application_settings, :dsa_key_restriction, :integer, null: false, + default: 0 + end +end diff --git a/db/post_migrate/20180523125103_cleanup_merge_requests_allow_maintainer_to_push_rename.rb b/db/post_migrate/20180523125103_cleanup_merge_requests_allow_maintainer_to_push_rename.rb new file mode 100644 index 00000000000..b9ce4600675 --- /dev/null +++ b/db/post_migrate/20180523125103_cleanup_merge_requests_allow_maintainer_to_push_rename.rb @@ -0,0 +1,15 @@ +class CleanupMergeRequestsAllowMaintainerToPushRename < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + cleanup_concurrent_column_rename :merge_requests, :allow_maintainer_to_push, :allow_collaboration + end + + def down + rename_column_concurrently :merge_requests, :allow_collaboration, :allow_maintainer_to_push + end +end diff --git a/db/schema.rb b/db/schema.rb index a6b0706b02a..b7d7cd89c14 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180529152628) do +ActiveRecord::Schema.define(version: 20180531220618) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -110,7 +110,7 @@ ActiveRecord::Schema.define(version: 20180529152628) do t.text "shared_runners_text_html" t.text "after_sign_up_text_html" t.integer "rsa_key_restriction", default: 0, null: false - t.integer "dsa_key_restriction", default: 0, null: false + t.integer "dsa_key_restriction", default: -1, null: false t.integer "ecdsa_key_restriction", default: 0, null: false t.integer "ed25519_key_restriction", default: 0, null: false t.boolean "housekeeping_enabled", default: true, null: false @@ -520,6 +520,7 @@ ActiveRecord::Schema.define(version: 20180529152628) do end add_index "ci_stages", ["pipeline_id", "name"], name: "index_ci_stages_on_pipeline_id_and_name", unique: true, using: :btree + add_index "ci_stages", ["pipeline_id", "position"], name: "index_ci_stages_on_pipeline_id_and_position", using: :btree add_index "ci_stages", ["pipeline_id"], name: "index_ci_stages_on_pipeline_id", using: :btree add_index "ci_stages", ["project_id"], name: "index_ci_stages_on_project_id", using: :btree @@ -1229,7 +1230,7 @@ ActiveRecord::Schema.define(version: 20180529152628) do t.boolean "discussion_locked" t.integer "latest_merge_request_diff_id" t.string "rebase_commit_sha" - t.boolean "allow_maintainer_to_push" + t.boolean "allow_collaboration" t.boolean "squash", default: false, null: false end diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 62f9884e264..9f06e20f803 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -651,7 +651,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_collaboration` | boolean | no | Allow commits from members who can merge to the target branch | +| `allow_maintainer_to_push` | boolean | no | Deprecated, see allow_collaboration | | `squash` | boolean | no | Squash commits into a single commit when merging | ```json @@ -709,6 +710,7 @@ POST /projects/:id/merge_requests "squash": false, "web_url": "http://example.com/example/example/merge_requests/1", "discussion_locked": false, + "allow_collaboration": false, "allow_maintainer_to_push": false, "time_stats": { "time_estimate": 0, @@ -741,7 +743,8 @@ PUT /projects/:id/merge_requests/:merge_request_iid | `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 | +| `allow_collaboration` | boolean | no | Allow commits from members who can merge to the target branch | +| `allow_maintainer_to_push` | boolean | no | Deprecated, see allow_collaboration | Must include at least one non-required attribute from above. @@ -799,6 +802,7 @@ Must include at least one non-required attribute from above. "squash": false, "web_url": "http://example.com/example/example/merge_requests/1", "discussion_locked": false, + "allow_collaboration": false, "allow_maintainer_to_push": false, "time_stats": { "time_estimate": 0, diff --git a/doc/api/pipelines.md b/doc/api/pipelines.md index 899f5da6647..ebae68fe389 100644 --- a/doc/api/pipelines.md +++ b/doc/api/pipelines.md @@ -102,6 +102,7 @@ POST /projects/:id/pipeline |------------|---------|----------|---------------------| | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | | `ref` | string | yes | Reference to commit | +| `variables` | array | no | An array containing the variables available in the pipeline, matching the structure [{ 'key' => 'UPLOAD_TO_S3', 'value' => 'true' }] | ``` curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/pipeline?ref=master" diff --git a/doc/development/i18n/proofreader.md b/doc/development/i18n/proofreader.md index 5185d843ccb..9a677bf09b2 100644 --- a/doc/development/i18n/proofreader.md +++ b/doc/development/i18n/proofreader.md @@ -16,7 +16,7 @@ are very appreciative of the work done by translators and proofreaders! - Dutch - Esperanto - French - - Rémy Coutable - [GitLab](https://gitlab.com/rymai), [Crowdin](https://crowdin.com/profile/rymai) + - Davy Defaud - [GitLab](https://gitlab.com/DevDef), [Crowdin](https://crowdin.com/profile/DevDef) - German - Indonesian - Ahmad Naufal Mukhtar - [GitLab](https://gitlab.com/anaufalm), [Crowdin](https://crowdin.com/profile/anaufalm) diff --git a/doc/user/project/container_registry.md b/doc/user/project/container_registry.md index 9c5e3509046..03302b3815d 100644 --- a/doc/user/project/container_registry.md +++ b/doc/user/project/container_registry.md @@ -143,6 +143,24 @@ docker login registry.example.com -u <your_username> -p <your_access_token> for errors (e.g. `/var/log/gitlab/gitlab-rails/production.log`). You may be able to find clues there. +#### Enable the registry debug server + +The optional debug server can be enabled by setting the registry debug address +in your `gitlab.rb` configuration. + +```ruby +registry['debug_addr'] = "localhost:5001" +``` + +After adding the setting, [reconfigure] GitLab to apply the change. + +Use curl to request debug output from the debug server: + +```bash +curl localhost:5001/debug/health +curl localhost:5001/debug/vars +``` + ### Advanced Troubleshooting >**NOTE:** The following section is only recommended for experts. @@ -275,3 +293,4 @@ Once the right permissions were set, the error will go away. [docker-docs]: https://docs.docker.com/engine/userguide/intro/ [pat]: ../profile/personal_access_tokens.md [pdt]: ../project/deploy_tokens/index.md +[reconfigure]: ../../administration/restart_gitlab.md#omnibus-gitlab-reconfigure
\ No newline at end of file diff --git a/doc/user/project/merge_requests/allow_collaboration.md b/doc/user/project/merge_requests/allow_collaboration.md new file mode 100644 index 00000000000..859ac92ef89 --- /dev/null +++ b/doc/user/project/merge_requests/allow_collaboration.md @@ -0,0 +1,20 @@ +# Allow collaboration on merge requests across forks + +> [Introduced][ce-17395] in GitLab 10.6. + +This feature is available for merge requests across forked projects that are +publicly accessible. It makes it easier for members of projects to +collaborate on merge requests across forks. + +When enabled for a merge request, members with merge access to the target +branch of the project will be granted write permissions to the source branch +of the merge request. + +The feature can only be enabled by users who already have push access to the +source project, and only lasts while the merge request is open. + +Enable this functionality while creating or editing a merge request: + + + +[ce-17395]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/17395 diff --git a/doc/user/project/merge_requests/img/allow_collaboration.png b/doc/user/project/merge_requests/img/allow_collaboration.png Binary files differnew file mode 100644 index 00000000000..75596e7d9ad --- /dev/null +++ b/doc/user/project/merge_requests/img/allow_collaboration.png diff --git a/doc/user/project/merge_requests/img/allow_maintainer_push.png b/doc/user/project/merge_requests/img/allow_maintainer_push.png Binary files differdeleted file mode 100644 index 91cc399f4ff..00000000000 --- a/doc/user/project/merge_requests/img/allow_maintainer_push.png +++ /dev/null diff --git a/doc/user/project/merge_requests/index.md b/doc/user/project/merge_requests/index.md index b75bcacc9d7..50979e82097 100644 --- a/doc/user/project/merge_requests/index.md +++ b/doc/user/project/merge_requests/index.md @@ -28,7 +28,7 @@ With GitLab merge requests, you can: - Enable [fast-forward merge requests](#fast-forward-merge-requests) - 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) +- [Allow collaboration](allow_collaboration.md) so members of the target project can push directly to the fork - [Squash and merge](squash_and_merge.md) for a cleaner commit history With **[GitLab Enterprise Edition][ee]**, you can also: diff --git a/doc/user/project/merge_requests/maintainer_access.md b/doc/user/project/merge_requests/maintainer_access.md index 89f71e16a50..d59afecd375 100644 --- a/doc/user/project/merge_requests/maintainer_access.md +++ b/doc/user/project/merge_requests/maintainer_access.md @@ -1,20 +1 @@ -# Allow maintainer pushes for merge requests across forks - -> [Introduced][ce-17395] in GitLab 10.6. - -This feature is available for merge requests across forked projects that are -publicly accessible. It makes it easier for maintainers of projects to -collaborate on merge requests across forks. - -When enabled for a merge request, members with merge access to the target -branch of the project will be granted write permissions to the source branch -of the merge request. - -The feature can only be enabled by users who already have push access to the -source project, and only lasts while the merge request is open. - -Enable this functionality while creating a merge request: - - - -[ce-17395]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/17395 +This document was moved to [another location](allow_collaboration.md). diff --git a/doc/user/project/pages/index.md b/doc/user/project/pages/index.md index 862e4a3b466..205f0283107 100644 --- a/doc/user/project/pages/index.md +++ b/doc/user/project/pages/index.md @@ -42,7 +42,7 @@ to secure them. Your files live in a project [repository](../repository/index.md) on GitLab. [GitLab CI](../../../ci/README.md) picks up those files and makes them available at, typically, -`http://<username>.gitlab.io/<projectname>`. Please read through the docs on +`https://<username>.gitlab.io/<projectname>`. Please read through the docs on [GitLab Pages domains](getting_started_part_one.md#gitlab-pages-domain) for more info. ## Explore GitLab Pages diff --git a/lib/api/entities.rb b/lib/api/entities.rb index c4537036a3a..c76d3ff45d0 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -559,7 +559,9 @@ module API expose :discussion_locked expose :should_remove_source_branch?, as: :should_remove_source_branch expose :force_remove_source_branch?, as: :force_remove_source_branch - expose :allow_maintainer_to_push, if: -> (merge_request, _) { merge_request.for_fork? } + expose :allow_collaboration, if: -> (merge_request, _) { merge_request.for_fork? } + # Deprecated + expose :allow_collaboration, as: :allow_maintainer_to_push, if: -> (merge_request, _) { merge_request.for_fork? } expose :web_url do |merge_request, options| Gitlab::UrlBuilder.build(merge_request) diff --git a/lib/api/events.rb b/lib/api/events.rb index b0713ff1d54..fc4ba5a3188 100644 --- a/lib/api/events.rb +++ b/lib/api/events.rb @@ -17,6 +17,7 @@ module API def present_events(events) events = events.reorder(created_at: params[:sort]) + .with_associations present paginate(events), with: Entities::Event end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 278d53427f0..af7d2471b34 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -162,7 +162,8 @@ 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 :allow_maintainer_to_push, type: Boolean, desc: 'Whether a maintainer of the target project can push to the source project' + optional :allow_collaboration, type: Boolean, desc: 'Allow commits from members who can merge to the target branch' + optional :allow_maintainer_to_push, type: Boolean, as: :allow_collaboration, desc: '[deprecated] See allow_collaboration' optional :squash, type: Grape::API::Boolean, desc: 'When true, the commits will be squashed into a single commit on merge' use :optional_params_ee diff --git a/lib/api/pipelines.rb b/lib/api/pipelines.rb index 735591fedd5..8374a57edfa 100644 --- a/lib/api/pipelines.rb +++ b/lib/api/pipelines.rb @@ -41,15 +41,20 @@ module API end params do requires :ref, type: String, desc: 'Reference' + optional :variables, Array, desc: 'Array of variables available in the pipeline' end post ':id/pipeline' do Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42124') authorize! :create_pipeline, user_project + pipeline_params = declared_params(include_missing: false) + .merge(variables_attributes: params[:variables]) + .except(:variables) + new_pipeline = Ci::CreatePipelineService.new(user_project, current_user, - declared_params(include_missing: false)) + pipeline_params) .execute(:api, ignore_skip_ci: true, save_on_errors: false) if new_pipeline.persisted? diff --git a/lib/backup.rb b/lib/backup.rb new file mode 100644 index 00000000000..e2c62af23ae --- /dev/null +++ b/lib/backup.rb @@ -0,0 +1,3 @@ +module Backup + Error = Class.new(StandardError) +end diff --git a/lib/backup/database.rb b/lib/backup/database.rb index 1608f7ad02d..086ca5986bd 100644 --- a/lib/backup/database.rb +++ b/lib/backup/database.rb @@ -44,7 +44,7 @@ module Backup end report_success(success) - abort 'Backup failed' unless success + raise Backup::Error, 'Backup failed' unless success end def restore @@ -72,7 +72,7 @@ module Backup end report_success(success) - abort 'Restore failed' unless success + abort Backup::Error, 'Restore failed' unless success end protected diff --git a/lib/backup/files.rb b/lib/backup/files.rb index 9895db9e451..d769a3ee7b0 100644 --- a/lib/backup/files.rb +++ b/lib/backup/files.rb @@ -26,7 +26,7 @@ module Backup unless status.zero? puts output - abort 'Backup failed' + raise Backup::Error, 'Backup failed' end run_pipeline!([%W(tar --exclude=lost+found -C #{@backup_files_dir} -cf - .), %w(gzip -c -1)], out: [backup_tarball, 'w', 0600]) @@ -39,7 +39,11 @@ module Backup def restore backup_existing_files_dir - run_pipeline!([%w(gzip -cd), %W(tar --unlink-first --recursive-unlink -C #{app_files_dir} -xf -)], in: backup_tarball) + run_pipeline!([%w(gzip -cd), %W(#{tar} --unlink-first --recursive-unlink -C #{app_files_dir} -xf -)], in: backup_tarball) + end + + def tar + system(*%w[gtar --version], out: '/dev/null') ? 'gtar' : 'tar' end def backup_existing_files_dir @@ -61,7 +65,7 @@ module Backup def run_pipeline!(cmd_list, options = {}) status_list = Open3.pipeline(*cmd_list, options) - abort 'Backup failed' unless status_list.compact.all?(&:success?) + raise Backup::Error, 'Backup failed' unless status_list.compact.all?(&:success?) end end end diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb index a8da0c7edef..a3641505196 100644 --- a/lib/backup/manager.rb +++ b/lib/backup/manager.rb @@ -27,7 +27,7 @@ module Backup progress.puts "done".color(:green) else puts "creating archive #{tar_file} failed".color(:red) - abort 'Backup failed' + raise Backup::Error, 'Backup failed' end upload @@ -52,7 +52,7 @@ module Backup progress.puts "done".color(:green) else puts "uploading backup to #{remote_directory} failed".color(:red) - abort 'Backup failed' + raise Backup::Error, 'Backup failed' end end @@ -66,7 +66,7 @@ module Backup progress.puts "done".color(:green) else puts "deleting tmp directory '#{dir}' failed".color(:red) - abort 'Backup failed' + raise Backup::Error, 'Backup failed' end end end diff --git a/lib/backup/repository.rb b/lib/backup/repository.rb index 84670d6582e..1b1c83d9fb3 100644 --- a/lib/backup/repository.rb +++ b/lib/backup/repository.rb @@ -17,7 +17,10 @@ module Backup Project.find_each(batch_size: 1000) do |project| progress.print " * #{display_repo_path(project)} ... " - path_to_project_repo = path_to_repo(project) + + path_to_project_repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + path_to_repo(project) + end path_to_project_bundle = path_to_bundle(project) # Create namespace dir or hashed path if missing @@ -51,7 +54,9 @@ module Backup end wiki = ProjectWiki.new(project) - path_to_wiki_repo = path_to_repo(wiki) + path_to_wiki_repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + path_to_repo(wiki) + end path_to_wiki_bundle = path_to_bundle(wiki) if File.exist?(path_to_wiki_repo) @@ -111,7 +116,9 @@ module Backup # TODO: Need to find a way to do this for gitaly # Gitaly migration issue: https://gitlab.com/gitlab-org/gitaly/issues/1195 in_path(path_to_tars(project)) do |dir| - path_to_project_repo = path_to_repo(project) + path_to_project_repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + path_to_repo(project) + end cmd = %W(tar -xf #{path_to_tars(project, dir)} -C #{path_to_project_repo} #{dir}) output, status = Gitlab::Popen.popen(cmd) diff --git a/lib/gitlab/ci/pipeline/preloader.rb b/lib/gitlab/ci/pipeline/preloader.rb index e7a2e5511cf..db0a1ea4dab 100644 --- a/lib/gitlab/ci/pipeline/preloader.rb +++ b/lib/gitlab/ci/pipeline/preloader.rb @@ -5,23 +5,47 @@ module Gitlab module Pipeline # Class for preloading data associated with pipelines such as commit # authors. - module Preloader - def self.preload(pipelines) - # This ensures that all the pipeline commits are eager loaded before we - # start using them. + class Preloader + def self.preload!(pipelines) + ## + # This preloads all commits at once, because `Ci::Pipeline#commit` is + # using a lazy batch loading, what results in only one batched Gitaly + # call. + # pipelines.each(&:commit) pipelines.each do |pipeline| - # This preloads the author of every commit. We're using "lazy_author" - # here since "author" immediately loads the data on the first call. - pipeline.commit.try(:lazy_author) - - # This preloads the number of warnings for every pipeline, ensuring - # that Ci::Pipeline#has_warnings? doesn't execute any additional - # queries. - pipeline.number_of_warnings + self.new(pipeline).tap do |preloader| + preloader.preload_commit_authors + preloader.preload_pipeline_warnings + preloader.preload_stages_warnings + end end end + + def initialize(pipeline) + @pipeline = pipeline + end + + def preload_commit_authors + # This also preloads the author of every commit. We're using "lazy_author" + # here since "author" immediately loads the data on the first call. + @pipeline.commit.try(:lazy_author) + end + + def preload_pipeline_warnings + # This preloads the number of warnings for every pipeline, ensuring + # that Ci::Pipeline#has_warnings? doesn't execute any additional + # queries. + @pipeline.number_of_warnings + end + + def preload_stages_warnings + # This preloads the number of warnings for every stage, ensuring + # that Ci::Stage#has_warnings? doesn't execute any additional + # queries. + @pipeline.stages.each { |stage| stage.number_of_warnings } + end end end end diff --git a/lib/gitlab/ci/status/stage/common.rb b/lib/gitlab/ci/status/stage/common.rb index bc99d925347..f60a7662075 100644 --- a/lib/gitlab/ci/status/stage/common.rb +++ b/lib/gitlab/ci/status/stage/common.rb @@ -8,7 +8,9 @@ module Gitlab end def details_path - project_pipeline_path(subject.project, subject.pipeline, anchor: subject.name) + project_pipeline_path(subject.pipeline.project, + subject.pipeline, + anchor: subject.name) end def has_action? diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 7acf11e3c91..d1b13ca2342 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -109,7 +109,7 @@ module Gitlab end def ==(other) - path == other.path + [storage, relative_path] == [other.storage, other.relative_path] end def path @@ -395,12 +395,12 @@ module Gitlab nil end - def archive_metadata(ref, storage_path, format = "tar.gz", append_sha:) + def archive_metadata(ref, storage_path, project_path, format = "tar.gz", append_sha:) ref ||= root_ref commit = Gitlab::Git::Commit.find(self, ref) return {} if commit.nil? - prefix = archive_prefix(ref, commit.id, append_sha: append_sha) + prefix = archive_prefix(ref, commit.id, project_path, append_sha: append_sha) { 'ArchivePrefix' => prefix, @@ -412,16 +412,12 @@ module Gitlab # This is both the filename of the archive (missing the extension) and the # name of the top-level member of the archive under which all files go - # - # FIXME: The generated prefix is incorrect for projects with hashed - # storage enabled - def archive_prefix(ref, sha, append_sha:) + def archive_prefix(ref, sha, project_path, append_sha:) append_sha = (ref != sha) if append_sha.nil? - project_name = self.name.chomp('.git') formatted_ref = ref.tr('/', '-') - prefix_segments = [project_name, formatted_ref] + prefix_segments = [project_path, formatted_ref] prefix_segments << sha if append_sha prefix_segments.join('-') diff --git a/lib/gitlab/gpg.rb b/lib/gitlab/gpg.rb index 413872d7e08..a4263369269 100644 --- a/lib/gitlab/gpg.rb +++ b/lib/gitlab/gpg.rb @@ -54,7 +54,11 @@ module Gitlab fingerprints = CurrentKeyChain.fingerprints_from_key(key) GPGME::Key.find(:public, fingerprints).flat_map do |raw_key| - raw_key.uids.map { |uid| { name: uid.name, email: uid.email.downcase } } + raw_key.uids.each_with_object([]) do |uid, arr| + name = uid.name.force_encoding('UTF-8') + email = uid.email.force_encoding('UTF-8') + arr << { name: name, email: email.downcase } if name.valid_encoding? && email.valid_encoding? + end end end end diff --git a/lib/gitlab/import_export/repo_saver.rb b/lib/gitlab/import_export/repo_saver.rb index 695462c7dd2..0c224bd1971 100644 --- a/lib/gitlab/import_export/repo_saver.rb +++ b/lib/gitlab/import_export/repo_saver.rb @@ -26,10 +26,6 @@ module Gitlab @shared.error(e) false end - - def path_to_repo - @project.repository.path_to_repo - end end end end diff --git a/lib/gitlab/import_export/wiki_repo_saver.rb b/lib/gitlab/import_export/wiki_repo_saver.rb index 5fa2e101e29..2fd62c0fc7b 100644 --- a/lib/gitlab/import_export/wiki_repo_saver.rb +++ b/lib/gitlab/import_export/wiki_repo_saver.rb @@ -22,12 +22,8 @@ module Gitlab "project.wiki.bundle" end - def path_to_repo - @wiki.repository.path_to_repo - end - def wiki_repository_exists? - File.exist?(@wiki.repository.path_to_repo) && !@wiki.repository.empty? + @wiki.repository.exists? && !@wiki.repository.empty? end end end diff --git a/lib/gitlab/task_helpers.rb b/lib/gitlab/task_helpers.rb index 42be301fd9b..723e655c150 100644 --- a/lib/gitlab/task_helpers.rb +++ b/lib/gitlab/task_helpers.rb @@ -128,10 +128,12 @@ module Gitlab end def all_repos - Gitlab.config.repositories.storages.each_value do |repository_storage| - IO.popen(%W(find #{repository_storage.legacy_disk_path} -mindepth 2 -type d -name *.git)) do |find| - find.each_line do |path| - yield path.chomp + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages.each_value do |repository_storage| + IO.popen(%W(find #{repository_storage.legacy_disk_path} -mindepth 2 -type d -name *.git)) do |find| + find.each_line do |path| + yield path.chomp + end end end end diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index 8cf5d636743..27560abfb96 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -65,7 +65,7 @@ module Gitlab return false unless can_access_git? return false unless project - return false if !user.can?(:push_code, project) && !project.branch_allows_maintainer_push?(user, ref) + return false if !user.can?(:push_code, project) && !project.branch_allows_collaboration?(user, ref) if protected?(ProtectedBranch, project, ref) protected_branch_accessible_to?(ref, action: :push) diff --git a/lib/tasks/gitlab/cleanup.rake b/lib/tasks/gitlab/cleanup.rake index d6d15285489..52ae1330d7f 100644 --- a/lib/tasks/gitlab/cleanup.rake +++ b/lib/tasks/gitlab/cleanup.rake @@ -12,7 +12,7 @@ namespace :gitlab do namespaces = Namespace.pluck(:path) namespaces << HASHED_REPOSITORY_NAME # add so that it will be ignored Gitlab.config.repositories.storages.each do |name, repository_storage| - git_base_path = repository_storage.legacy_disk_path + git_base_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository_storage.legacy_disk_path } all_dirs = Dir.glob(git_base_path + '/*') puts git_base_path.color(:yellow) @@ -54,7 +54,8 @@ namespace :gitlab do move_suffix = "+orphaned+#{Time.now.to_i}" Gitlab.config.repositories.storages.each do |name, repository_storage| - repo_root = repository_storage.legacy_disk_path + repo_root = Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository_storage.legacy_disk_path } + # Look for global repos (legacy, depth 1) and normal repos (depth 2) IO.popen(%W(find #{repo_root} -mindepth 1 -maxdepth 2 -name *.git)) do |find| find.each_line do |path| diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 035a2275d9f..8ae04cd2f88 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -331,7 +331,7 @@ msgstr "" msgid "All features are enabled for blank projects, from templates, or when importing, but you can disable them afterward in the project settings." msgstr "" -msgid "Allow edits from maintainers." +msgid "Allow commits from members who can merge to the target branch." msgstr "" msgid "Allow rendering of PlantUML diagrams in Asciidoc documents." @@ -4894,7 +4894,7 @@ msgstr "" msgid "mrWidget|%{metricsLinkStart} Memory %{metricsLinkEnd} usage is %{emphasisStart} unchanged %{emphasisEnd} at %{memoryFrom}MB" msgstr "" -msgid "mrWidget|Allows edits from maintainers" +msgid "mrWidget|Allows commits from members who can merge to the target branch" msgstr "" msgid "mrWidget|Cancel automatic merge" diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index b048da1991c..683c57c96f8 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -477,4 +477,28 @@ describe ApplicationController do end end end + + describe '#access_denied' do + controller(described_class) do + def index + access_denied!(params[:message]) + end + end + + before do + sign_in user + end + + it 'renders a 404 without a message' do + get :index + + expect(response).to have_gitlab_http_status(404) + end + + it 'renders a 403 when a message is passed to access denied' do + get :index, message: 'None shall pass' + + expect(response).to have_gitlab_http_status(403) + end + end end diff --git a/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb b/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb index 27f558e1b5d..d20471ef603 100644 --- a/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb +++ b/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb @@ -43,13 +43,13 @@ describe ControllerWithCrossProjectAccessCheck do end end - it 'renders a 404 with trying to access a cross project page' do + it 'renders a 403 with trying to access a cross project page' do message = "This page is unavailable because you are not allowed to read "\ "information across multiple projects." get :index - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) expect(response.body).to match(/#{message}/) end @@ -119,7 +119,7 @@ describe ControllerWithCrossProjectAccessCheck do get :index - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end it 'is executed when the `unless` condition returns true' do @@ -127,19 +127,19 @@ describe ControllerWithCrossProjectAccessCheck do get :index - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end it 'does not skip the check on an action that is not skipped' do get :show, id: 'hello' - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end it 'does not skip the check on an action that was not defined to skip' do get :edit, id: 'hello' - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end end end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 9e7bc20a6d1..92886e93077 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -17,44 +17,103 @@ describe Projects::PipelinesController do describe 'GET index.json' do before do - %w(pending running created success).each_with_index do |status, index| - sha = project.commit("HEAD~#{index}") - create(:ci_empty_pipeline, status: status, project: project, sha: sha) + %w(pending running success failed canceled).each_with_index do |status, index| + create_pipeline(status, project.commit("HEAD~#{index}")) end end - subject do - get :index, namespace_id: project.namespace, project_id: project, format: :json + context 'when using persisted stages', :request_store do + before do + stub_feature_flags(ci_pipeline_persisted_stages: true) + end + + it 'returns serialized pipelines', :request_store do + queries = ActiveRecord::QueryRecorder.new do + get_pipelines_index_json + end + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('pipeline') + + expect(json_response).to include('pipelines') + expect(json_response['pipelines'].count).to eq 5 + expect(json_response['count']['all']).to eq '5' + expect(json_response['count']['running']).to eq '1' + expect(json_response['count']['pending']).to eq '1' + expect(json_response['count']['finished']).to eq '3' + + json_response.dig('pipelines', 0, 'details', 'stages').tap do |stages| + expect(stages.count).to eq 3 + end + + expect(queries.count).to be + end end - it 'returns JSON with serialized pipelines' do - subject + context 'when using legacy stages', :request_store do + before do + stub_feature_flags(ci_pipeline_persisted_stages: false) + end - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('pipeline') + it 'returns JSON with serialized pipelines', :request_store do + queries = ActiveRecord::QueryRecorder.new do + get_pipelines_index_json + end + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('pipeline') - expect(json_response).to include('pipelines') - expect(json_response['pipelines'].count).to eq 4 - expect(json_response['count']['all']).to eq '4' - expect(json_response['count']['running']).to eq '1' - expect(json_response['count']['pending']).to eq '1' - expect(json_response['count']['finished']).to eq '1' + expect(json_response).to include('pipelines') + expect(json_response['pipelines'].count).to eq 5 + expect(json_response['count']['all']).to eq '5' + expect(json_response['count']['running']).to eq '1' + expect(json_response['count']['pending']).to eq '1' + expect(json_response['count']['finished']).to eq '3' + + json_response.dig('pipelines', 0, 'details', 'stages').tap do |stages| + expect(stages.count).to eq 3 + end + + expect(queries.count).to be_within(3).of(30) + end end it 'does not include coverage data for the pipelines' do - subject + get_pipelines_index_json expect(json_response['pipelines'][0]).not_to include('coverage') end context 'when performing gitaly calls', :request_store do it 'limits the Gitaly requests' do - expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(3) + expect { get_pipelines_index_json } + .to change { Gitlab::GitalyClient.get_request_count }.by(2) end end + + def get_pipelines_index_json + get :index, namespace_id: project.namespace, + project_id: project, + format: :json + end + + def create_pipeline(status, sha) + pipeline = create(:ci_empty_pipeline, status: status, + project: project, + sha: sha) + + create_build(pipeline, 'build', 1, 'build') + create_build(pipeline, 'test', 2, 'test') + create_build(pipeline, 'deploy', 3, 'deploy') + end + + def create_build(pipeline, stage, stage_idx, name) + status = %w[created running pending success failed canceled].sample + create(:ci_build, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name, status: status) + end end - describe 'GET show JSON' do + describe 'GET show.json' do let(:pipeline) { create(:ci_pipeline_with_one_job, project: project) } it 'returns the pipeline' do @@ -67,6 +126,14 @@ describe Projects::PipelinesController do end context 'when the pipeline has multiple stages and groups', :request_store do + let(:project) { create(:project, :repository) } + + let(:pipeline) do + create(:ci_empty_pipeline, project: project, + user: user, + sha: project.commit.id) + end + before do create_build('build', 0, 'build') create_build('test', 1, 'rspec 0') @@ -74,11 +141,6 @@ describe Projects::PipelinesController do create_build('post deploy', 3, 'pages 0') end - let(:project) { create(:project, :repository) } - let(:pipeline) do - create(:ci_empty_pipeline, project: project, user: user, sha: project.commit.id) - end - it 'does not perform N + 1 queries' do control_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count @@ -90,6 +152,7 @@ describe Projects::PipelinesController do create_build('post deploy', 3, 'pages 2') new_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count + expect(new_count).to be_within(12).of(control_count) end end diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 30c06ddf744..416a09e1684 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -32,7 +32,7 @@ describe SearchController do it 'still blocks searches without a project_id' do get :show, search: 'hello' - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(403) end it 'allows searches with a project_id' do diff --git a/spec/controllers/users/terms_controller_spec.rb b/spec/controllers/users/terms_controller_spec.rb index a744463413c..0d77e91a67d 100644 --- a/spec/controllers/users/terms_controller_spec.rb +++ b/spec/controllers/users/terms_controller_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' describe Users::TermsController do + include TermsHelper let(:user) { create(:user) } let(:term) { create(:term) } @@ -15,10 +16,25 @@ describe Users::TermsController do expect(response).to have_gitlab_http_status(:redirect) end - it 'shows terms when they exist' do - term + context 'when terms exist' do + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + term + end + + it 'shows terms when they exist' do + get :index + + expect(response).to have_gitlab_http_status(:success) + end - expect(response).to have_gitlab_http_status(:success) + it 'shows a message when the user already accepted the terms' do + accept_terms(user) + + get :index + + expect(controller).to set_flash.now[:notice].to(/already accepted/) + end end end diff --git a/spec/factories/term_agreements.rb b/spec/factories/term_agreements.rb index 557599e663d..3c4eebd0196 100644 --- a/spec/factories/term_agreements.rb +++ b/spec/factories/term_agreements.rb @@ -3,4 +3,12 @@ FactoryBot.define do term user end + + trait :declined do + accepted false + end + + trait :accepted do + accepted true + end end diff --git a/spec/features/markdown/markdown_spec.rb b/spec/features/markdown/markdown_spec.rb index f13d78d24e3..c86ba8c50a5 100644 --- a/spec/features/markdown/markdown_spec.rb +++ b/spec/features/markdown/markdown_spec.rb @@ -24,7 +24,7 @@ require 'erb' # # See the MarkdownFeature class for setup details. -describe 'GitLab Markdown' do +describe 'GitLab Markdown', :aggregate_failures do include Capybara::Node::Matchers include MarkupHelper include MarkdownMatchers @@ -44,112 +44,102 @@ describe 'GitLab Markdown' do # Shared behavior that all pipelines should exhibit shared_examples 'all pipelines' do - describe 'Redcarpet extensions' do - it 'does not parse emphasis inside of words' do + it 'includes Redcarpet extensions' do + aggregate_failures 'does not parse emphasis inside of words' do expect(doc.to_html).not_to match('foo<em>bar</em>baz') end - it 'parses table Markdown' do - aggregate_failures do - expect(doc).to have_selector('th:contains("Header")') - expect(doc).to have_selector('th:contains("Row")') - expect(doc).to have_selector('th:contains("Example")') - end + aggregate_failures 'parses table Markdown' do + expect(doc).to have_selector('th:contains("Header")') + expect(doc).to have_selector('th:contains("Row")') + expect(doc).to have_selector('th:contains("Example")') end - it 'allows Markdown in tables' do + aggregate_failures 'allows Markdown in tables' do expect(doc.at_css('td:contains("Baz")').children.to_html) .to eq '<strong>Baz</strong>' end - it 'parses fenced code blocks' do - aggregate_failures do - expect(doc).to have_selector('pre.code.highlight.js-syntax-highlight.c') - expect(doc).to have_selector('pre.code.highlight.js-syntax-highlight.python') - end + aggregate_failures 'parses fenced code blocks' do + expect(doc).to have_selector('pre.code.highlight.js-syntax-highlight.c') + expect(doc).to have_selector('pre.code.highlight.js-syntax-highlight.python') end - it 'parses mermaid code block' do - aggregate_failures do - expect(doc).to have_selector('pre[lang=mermaid] > code.js-render-mermaid') - end + aggregate_failures 'parses mermaid code block' do + expect(doc).to have_selector('pre[lang=mermaid] > code.js-render-mermaid') end - it 'parses strikethroughs' do + aggregate_failures 'parses strikethroughs' do expect(doc).to have_selector(%{del:contains("and this text doesn't")}) end - it 'parses superscript' do + aggregate_failures 'parses superscript' do expect(doc).to have_selector('sup', count: 2) end end - describe 'SanitizationFilter' do - it 'permits b elements' do + it 'includes SanitizationFilter' do + aggregate_failures 'permits b elements' do expect(doc).to have_selector('b:contains("b tag")') end - it 'permits em elements' do + aggregate_failures 'permits em elements' do expect(doc).to have_selector('em:contains("em tag")') end - it 'permits code elements' do + aggregate_failures 'permits code elements' do expect(doc).to have_selector('code:contains("code tag")') end - it 'permits kbd elements' do + aggregate_failures 'permits kbd elements' do expect(doc).to have_selector('kbd:contains("s")') end - it 'permits strike elements' do + aggregate_failures 'permits strike elements' do expect(doc).to have_selector('strike:contains(Emoji)') end - it 'permits img elements' do + aggregate_failures 'permits img elements' do expect(doc).to have_selector('img[data-src*="smile.png"]') end - it 'permits br elements' do + aggregate_failures 'permits br elements' do expect(doc).to have_selector('br') end - it 'permits hr elements' do + aggregate_failures 'permits hr elements' do expect(doc).to have_selector('hr') end - it 'permits span elements' do + aggregate_failures 'permits span elements' do expect(doc).to have_selector('span:contains("span tag")') end - it 'permits details elements' do + aggregate_failures 'permits details elements' do expect(doc).to have_selector('details:contains("Hiding the details")') end - it 'permits summary elements' do + aggregate_failures 'permits summary elements' do expect(doc).to have_selector('details summary:contains("collapsible")') end - it 'permits style attribute in th elements' do - aggregate_failures do - expect(doc.at_css('th:contains("Header")')['style']).to eq 'text-align: center' - expect(doc.at_css('th:contains("Row")')['style']).to eq 'text-align: right' - expect(doc.at_css('th:contains("Example")')['style']).to eq 'text-align: left' - end + aggregate_failures 'permits style attribute in th elements' do + expect(doc.at_css('th:contains("Header")')['style']).to eq 'text-align: center' + expect(doc.at_css('th:contains("Row")')['style']).to eq 'text-align: right' + expect(doc.at_css('th:contains("Example")')['style']).to eq 'text-align: left' end - it 'permits style attribute in td elements' do - aggregate_failures do - expect(doc.at_css('td:contains("Foo")')['style']).to eq 'text-align: center' - expect(doc.at_css('td:contains("Bar")')['style']).to eq 'text-align: right' - expect(doc.at_css('td:contains("Baz")')['style']).to eq 'text-align: left' - end + aggregate_failures 'permits style attribute in td elements' do + expect(doc.at_css('td:contains("Foo")')['style']).to eq 'text-align: center' + expect(doc.at_css('td:contains("Bar")')['style']).to eq 'text-align: right' + expect(doc.at_css('td:contains("Baz")')['style']).to eq 'text-align: left' end - it 'removes `rel` attribute from links' do + aggregate_failures 'removes `rel` attribute from links' do expect(doc).not_to have_selector('a[rel="bookmark"]') end - it "removes `href` from `a` elements if it's fishy" do + aggregate_failures "removes `href` from `a` elements if it's fishy" do expect(doc).not_to have_selector('a[href*="javascript"]') end end @@ -176,26 +166,26 @@ describe 'GitLab Markdown' do end end - describe 'ExternalLinkFilter' do - it 'adds nofollow to external link' do + it 'includes ExternalLinkFilter' do + aggregate_failures 'adds nofollow to external link' do link = doc.at_css('a:contains("Google")') expect(link.attr('rel')).to include('nofollow') end - it 'adds noreferrer to external link' do + aggregate_failures 'adds noreferrer to external link' do link = doc.at_css('a:contains("Google")') expect(link.attr('rel')).to include('noreferrer') end - it 'adds _blank to target attribute for external links' do + aggregate_failures 'adds _blank to target attribute for external links' do link = doc.at_css('a:contains("Google")') expect(link.attr('target')).to match('_blank') end - it 'ignores internal link' do + aggregate_failures 'ignores internal link' do link = doc.at_css('a:contains("GitLab Root")') expect(link.attr('rel')).not_to match 'nofollow' @@ -219,24 +209,24 @@ describe 'GitLab Markdown' do it_behaves_like 'all pipelines' - it 'includes RelativeLinkFilter' do - expect(doc).to parse_relative_links - end + it 'includes custom filters' do + aggregate_failures 'RelativeLinkFilter' do + expect(doc).to parse_relative_links + end - it 'includes EmojiFilter' do - expect(doc).to parse_emoji - end + aggregate_failures 'EmojiFilter' do + expect(doc).to parse_emoji + end - it 'includes TableOfContentsFilter' do - expect(doc).to create_header_links - end + aggregate_failures 'TableOfContentsFilter' do + expect(doc).to create_header_links + end - it 'includes AutolinkFilter' do - expect(doc).to create_autolinks - end + aggregate_failures 'AutolinkFilter' do + expect(doc).to create_autolinks + end - it 'includes all reference filters' do - aggregate_failures do + aggregate_failures 'all reference filters' do expect(doc).to reference_users expect(doc).to reference_issues expect(doc).to reference_merge_requests @@ -246,22 +236,22 @@ describe 'GitLab Markdown' do expect(doc).to reference_labels expect(doc).to reference_milestones end - end - it 'includes TaskListFilter' do - expect(doc).to parse_task_lists - end + aggregate_failures 'TaskListFilter' do + expect(doc).to parse_task_lists + end - it 'includes InlineDiffFilter' do - expect(doc).to parse_inline_diffs - end + aggregate_failures 'InlineDiffFilter' do + expect(doc).to parse_inline_diffs + end - it 'includes VideoLinkFilter' do - expect(doc).to parse_video_links - end + aggregate_failures 'VideoLinkFilter' do + expect(doc).to parse_video_links + end - it 'includes ColorFilter' do - expect(doc).to parse_colors + aggregate_failures 'ColorFilter' do + expect(doc).to parse_colors + end end end @@ -280,24 +270,24 @@ describe 'GitLab Markdown' do it_behaves_like 'all pipelines' - it 'includes RelativeLinkFilter' do - expect(doc).not_to parse_relative_links - end + it 'includes custom filters' do + aggregate_failures 'RelativeLinkFilter' do + expect(doc).not_to parse_relative_links + end - it 'includes EmojiFilter' do - expect(doc).to parse_emoji - end + aggregate_failures 'EmojiFilter' do + expect(doc).to parse_emoji + end - it 'includes TableOfContentsFilter' do - expect(doc).to create_header_links - end + aggregate_failures 'TableOfContentsFilter' do + expect(doc).to create_header_links + end - it 'includes AutolinkFilter' do - expect(doc).to create_autolinks - end + aggregate_failures 'AutolinkFilter' do + expect(doc).to create_autolinks + end - it 'includes all reference filters' do - aggregate_failures do + aggregate_failures 'all reference filters' do expect(doc).to reference_users expect(doc).to reference_issues expect(doc).to reference_merge_requests @@ -307,26 +297,26 @@ describe 'GitLab Markdown' do expect(doc).to reference_labels expect(doc).to reference_milestones end - end - it 'includes TaskListFilter' do - expect(doc).to parse_task_lists - end + aggregate_failures 'TaskListFilter' do + expect(doc).to parse_task_lists + end - it 'includes GollumTagsFilter' do - expect(doc).to parse_gollum_tags - end + aggregate_failures 'GollumTagsFilter' do + expect(doc).to parse_gollum_tags + end - it 'includes InlineDiffFilter' do - expect(doc).to parse_inline_diffs - end + aggregate_failures 'InlineDiffFilter' do + expect(doc).to parse_inline_diffs + end - it 'includes VideoLinkFilter' do - expect(doc).to parse_video_links - end + aggregate_failures 'VideoLinkFilter' do + expect(doc).to parse_video_links + end - it 'includes ColorFilter' do - expect(doc).to parse_colors + aggregate_failures 'ColorFilter' do + expect(doc).to parse_colors + end end end diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb index a3323da1b1f..1808d0c0a0c 100644 --- a/spec/features/merge_request/maintainer_edits_fork_spec.rb +++ b/spec/features/merge_request/maintainer_edits_fork_spec.rb @@ -14,7 +14,7 @@ describe 'a maintainer edits files on a source-branch of an MR from a fork', :js source_branch: 'fix', target_branch: 'master', author: author, - allow_maintainer_to_push: true) + allow_collaboration: true) end before do diff --git a/spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb b/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb index eb41d7de8ed..0af37d76539 100644 --- a/spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb +++ b/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'create a merge request that allows maintainers to push', :js do +describe 'create a merge request, allowing commits from members who can merge to the target branch', :js do include ProjectForksHelper let(:user) { create(:user) } let(:target_project) { create(:project, :public, :repository) } @@ -21,16 +21,16 @@ describe 'create a merge request that allows maintainers to push', :js do sign_in(user) end - it 'allows setting maintainer push possible' do + it 'allows setting possible' do visit_new_merge_request - check 'Allow edits from maintainers' + check 'Allow commits from members who can merge to the target branch' click_button 'Submit merge request' wait_for_requests - expect(page).to have_content('Allows edits from maintainers') + expect(page).to have_content('Allows commits from members who can merge to the target branch') end it 'shows a message when one of the projects is private' do @@ -57,12 +57,12 @@ describe 'create a merge request that allows maintainers to push', :js do visit_new_merge_request - expect(page).not_to have_content('Allows edits from maintainers') + expect(page).not_to have_content('Allows commits from members who can merge to the target branch') end end - context 'when a maintainer tries to edit the option' do - let(:maintainer) { create(:user) } + context 'when a member who can merge tries to edit the option' do + let(:member) { create(:user) } let(:merge_request) do create(:merge_request, source_project: source_project, @@ -71,15 +71,15 @@ describe 'create a merge request that allows maintainers to push', :js do end before do - target_project.add_master(maintainer) + target_project.add_master(member) - sign_in(maintainer) + sign_in(member) end - it 'it hides the option from maintainers' do + it 'it hides the option from members' do visit edit_project_merge_request_path(target_project, merge_request) - expect(page).not_to have_content('Allows edits from maintainers') + expect(page).not_to have_content('Allows commits from members who can merge to the target branch') end end end diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb index 60fe30bd898..d0912e645bc 100644 --- a/spec/features/projects/import_export/import_file_spec.rb +++ b/spec/features/projects/import_export/import_file_spec.rb @@ -87,11 +87,13 @@ feature 'Import/Export - project import integration test', :js do def wiki_exists?(project) wiki = ProjectWiki.new(project) - File.exist?(wiki.repository.path_to_repo) && !wiki.repository.empty? + wiki.repository.exists? && !wiki.repository.empty? end def project_hook_exists?(project) - Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository).exists? + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository).exists? + end end def click_import_project_tab diff --git a/spec/features/users/terms_spec.rb b/spec/features/users/terms_spec.rb index 1efa5cd5490..af407c52917 100644 --- a/spec/features/users/terms_spec.rb +++ b/spec/features/users/terms_spec.rb @@ -39,6 +39,22 @@ describe 'Users > Terms' do end end + context 'when the user has already accepted the terms' do + before do + accept_terms(user) + end + + it 'allows the user to continue to the app' do + visit terms_path + + expect(page).to have_content "You have already accepted the Terms of Service as #{user.to_reference}" + + click_link 'Continue' + + expect(current_path).to eq(root_path) + end + end + context 'terms were enforced while session is active', :js do let(:project) { create(:project) } diff --git a/spec/fixtures/api/schemas/entities/merge_request_basic.json b/spec/fixtures/api/schemas/entities/merge_request_basic.json index 46031961cca..f7bc137c90c 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_basic.json +++ b/spec/fixtures/api/schemas/entities/merge_request_basic.json @@ -13,6 +13,7 @@ "assignee_id": { "type": ["integer", "null"] }, "subscribed": { "type": ["boolean", "null"] }, "participants": { "type": "array" }, + "allow_collaboration": { "type": "boolean"}, "allow_maintainer_to_push": { "type": "boolean"} }, "additionalProperties": false diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index 7be8c9e3e67..ee5588fa6c6 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -31,7 +31,7 @@ "source_project_id": { "type": "integer" }, "target_branch": { "type": "string" }, "target_project_id": { "type": "integer" }, - "allow_maintainer_to_push": { "type": "boolean"}, + "allow_collaboration": { "type": "boolean"}, "metrics": { "oneOf": [ { "type": "null" }, 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 f97461ce9cc..f7adc4e0b91 100644 --- a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json +++ b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json @@ -82,6 +82,7 @@ "human_time_estimate": { "type": ["string", "null"] }, "human_total_time_spent": { "type": ["string", "null"] } }, + "allow_collaboration": { "type": ["boolean", "null"] }, "allow_maintainer_to_push": { "type": ["boolean", "null"] } }, "required": [ diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 4e5391295b6..d372e58f63d 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -5,9 +5,9 @@ describe ProjectsHelper do describe "#project_status_css_class" do it "returns appropriate class" do - expect(project_status_css_class("started")).to eq("active") - expect(project_status_css_class("failed")).to eq("danger") - expect(project_status_css_class("finished")).to eq("success") + expect(project_status_css_class("started")).to eq("table-active") + expect(project_status_css_class("failed")).to eq("table-danger") + expect(project_status_css_class("finished")).to eq("table-success") end end diff --git a/spec/initializers/fog_google_https_private_urls_spec.rb b/spec/initializers/fog_google_https_private_urls_spec.rb index de3c157ab7b..08346b71fee 100644 --- a/spec/initializers/fog_google_https_private_urls_spec.rb +++ b/spec/initializers/fog_google_https_private_urls_spec.rb @@ -1,13 +1,13 @@ require 'spec_helper' -describe 'Fog::Storage::GoogleXML::File' do +describe 'Fog::Storage::GoogleXML::File', :fog_requests do let(:storage) do Fog.mock! - Fog::Storage.new({ - google_storage_access_key_id: "asdf", - google_storage_secret_access_key: "asdf", - provider: "Google" - }) + Fog::Storage.new( + google_storage_access_key_id: "asdf", + google_storage_secret_access_key: "asdf", + provider: "Google" + ) end let(:file) do diff --git a/spec/javascripts/importer_status_spec.js b/spec/javascripts/importer_status_spec.js index 0575d02886d..87b46ccf7c3 100644 --- a/spec/javascripts/importer_status_spec.js +++ b/spec/javascripts/importer_status_spec.js @@ -45,7 +45,7 @@ describe('Importer Status', () => { currentTarget: document.querySelector('.js-add-to-import'), }) .then(() => { - expect(document.querySelector('tr').classList.contains('active')).toEqual(true); + expect(document.querySelector('tr').classList.contains('table-active')).toEqual(true); done(); }) .catch(done.fail); diff --git a/spec/javascripts/vue_shared/components/gl_modal_spec.js b/spec/javascripts/vue_shared/components/gl_modal_spec.js index 85cb1b90fc6..23be8d93b81 100644 --- a/spec/javascripts/vue_shared/components/gl_modal_spec.js +++ b/spec/javascripts/vue_shared/components/gl_modal_spec.js @@ -190,4 +190,37 @@ describe('GlModal', () => { }); }); }); + + describe('handling sizes', () => { + it('should render modal-sm', () => { + vm = mountComponent(modalComponent, { + modalSize: 'sm', + }); + + expect(vm.$el.querySelector('.modal-dialog').classList.contains('modal-sm')).toEqual(true); + }); + + it('should render modal-lg', () => { + vm = mountComponent(modalComponent, { + modalSize: 'lg', + }); + + expect(vm.$el.querySelector('.modal-dialog').classList.contains('modal-lg')).toEqual(true); + }); + + it('should not add modal size classes when md size is passed', () => { + vm = mountComponent(modalComponent, { + modalSize: 'md', + }); + + expect(vm.$el.querySelector('.modal-dialog').classList.contains('modal-md')).toEqual(false); + }); + + it('should not add modal size classes by default', () => { + vm = mountComponent(modalComponent, {}); + + expect(vm.$el.querySelector('.modal-dialog').classList.contains('modal-sm')).toEqual(false); + expect(vm.$el.querySelector('.modal-dialog').classList.contains('modal-lg')).toEqual(false); + }); + }); }); diff --git a/spec/javascripts/vue_shared/components/table_pagination_spec.js b/spec/javascripts/vue_shared/components/table_pagination_spec.js index c63f15e5880..c36b607a34e 100644 --- a/spec/javascripts/vue_shared/components/table_pagination_spec.js +++ b/spec/javascripts/vue_shared/components/table_pagination_spec.js @@ -51,7 +51,7 @@ describe('Pagination component', () => { expect( component.$el.querySelector('.js-previous-button').classList.contains('disabled'), - ).toEqual(true); + ).toEqual(true); component.$el.querySelector('.js-previous-button a').click(); diff --git a/spec/lib/backup/files_spec.rb b/spec/lib/backup/files_spec.rb index 99872211a4e..63f2298357f 100644 --- a/spec/lib/backup/files_spec.rb +++ b/spec/lib/backup/files_spec.rb @@ -46,7 +46,9 @@ describe Backup::Files do end it 'calls tar command with unlink' do - expect(subject).to receive(:run_pipeline!).with([%w(gzip -cd), %w(tar --unlink-first --recursive-unlink -C /var/gitlab-registry -xf -)], any_args) + expect(subject).to receive(:tar).and_return('blabla-tar') + + expect(subject).to receive(:run_pipeline!).with([%w(gzip -cd), %w(blabla-tar --unlink-first --recursive-unlink -C /var/gitlab-registry -xf -)], any_args) subject.restore end end diff --git a/spec/lib/backup/manager_spec.rb b/spec/lib/backup/manager_spec.rb index 23c04a1a101..ca319679e80 100644 --- a/spec/lib/backup/manager_spec.rb +++ b/spec/lib/backup/manager_spec.rb @@ -274,16 +274,13 @@ describe Backup::Manager do } ) - # the Fog mock only knows about directories we create explicitly Fog.mock! + + # the Fog mock only knows about directories we create explicitly connection = ::Fog::Storage.new(Gitlab.config.backup.upload.connection.symbolize_keys) connection.directories.create(key: Gitlab.config.backup.upload.remote_directory) end - after do - Fog.unmock! - end - context 'target path' do it 'uses the tar filename by default' do expect_any_instance_of(Fog::Collection).to receive(:create) diff --git a/spec/lib/backup/repository_spec.rb b/spec/lib/backup/repository_spec.rb index f583b2021a2..92a27e308d2 100644 --- a/spec/lib/backup/repository_spec.rb +++ b/spec/lib/backup/repository_spec.rb @@ -34,7 +34,9 @@ describe Backup::Repository do let(:timestamp) { Time.utc(2017, 3, 22) } let(:temp_dirs) do Gitlab.config.repositories.storages.map do |name, storage| - File.join(storage.legacy_disk_path, '..', 'repositories.old.' + timestamp.to_i.to_s) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join(storage.legacy_disk_path, '..', 'repositories.old.' + timestamp.to_i.to_s) + end end end diff --git a/spec/lib/gitlab/bare_repository_import/importer_spec.rb b/spec/lib/gitlab/bare_repository_import/importer_spec.rb index 5c8a19a53bc..468f6ff6d24 100644 --- a/spec/lib/gitlab/bare_repository_import/importer_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/importer_spec.rb @@ -20,6 +20,13 @@ describe Gitlab::BareRepositoryImport::Importer, repository: true do Rainbow.enabled = @rainbow end + around do |example| + # TODO migrate BareRepositoryImport https://gitlab.com/gitlab-org/gitaly/issues/953 + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + shared_examples 'importing a repository' do describe '.execute' do it 'creates a project for a repository in storage' do diff --git a/spec/lib/gitlab/bare_repository_import/repository_spec.rb b/spec/lib/gitlab/bare_repository_import/repository_spec.rb index 1504826c7a5..afd8f5da39f 100644 --- a/spec/lib/gitlab/bare_repository_import/repository_spec.rb +++ b/spec/lib/gitlab/bare_repository_import/repository_spec.rb @@ -62,8 +62,10 @@ describe ::Gitlab::BareRepositoryImport::Repository do before do gitlab_shell.create_repository(repository_storage, hashed_path) - repository = Rugged::Repository.new(repo_path) - repository.config['gitlab.fullpath'] = 'to/repo' + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository = Rugged::Repository.new(repo_path) + repository.config['gitlab.fullpath'] = 'to/repo' + end end after do diff --git a/spec/lib/gitlab/ci/pipeline/preloader_spec.rb b/spec/lib/gitlab/ci/pipeline/preloader_spec.rb index 477c7477df0..40dfd893465 100644 --- a/spec/lib/gitlab/ci/pipeline/preloader_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/preloader_spec.rb @@ -3,18 +3,47 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Preloader do - describe '.preload' do - it 'preloads the author of every pipeline commit' do - commit = double(:commit) - pipeline = double(:pipeline, commit: commit) + let(:stage) { double(:stage) } + let(:commit) { double(:commit) } - expect(commit) - .to receive(:lazy_author) + let(:pipeline) do + double(:pipeline, commit: commit, stages: [stage]) + end + + describe '.preload!' do + context 'when preloading multiple commits' do + let(:project) { create(:project, :repository) } + + it 'preloads all commits once' do + expect(Commit).to receive(:decorate).once.and_call_original + + pipelines = [build_pipeline(ref: 'HEAD'), + build_pipeline(ref: 'HEAD~1')] + + described_class.preload!(pipelines) + end + + def build_pipeline(ref:) + build_stubbed(:ci_pipeline, project: project, sha: project.commit(ref).id) + end + end + + it 'preloads commit authors and number of warnings' do + expect(commit).to receive(:lazy_author) + expect(pipeline).to receive(:number_of_warnings) + expect(stage).to receive(:number_of_warnings) + + described_class.preload!([pipeline]) + end + + it 'returns original collection' do + allow(commit).to receive(:lazy_author) + allow(pipeline).to receive(:number_of_warnings) + allow(stage).to receive(:number_of_warnings) - expect(pipeline) - .to receive(:number_of_warnings) + pipelines = [pipeline, pipeline] - described_class.preload([pipeline]) + expect(described_class.preload!(pipelines)).to eq pipelines end end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 20b0b2c53a0..1744db1b17e 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -159,6 +159,7 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:feature2) { 'feature2' } around do |example| + # discover_default_branch will be moved to gitaly-ruby Gitlab::GitalyClient::StorageSettings.allow_disk_access do example.run end @@ -255,7 +256,7 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:expected_path) { File.join(storage_path, cache_key, expected_filename) } let(:expected_prefix) { "gitlab-git-test-#{ref}-#{SeedRepo::LastCommit::ID}" } - subject(:metadata) { repository.archive_metadata(ref, storage_path, format, append_sha: append_sha) } + subject(:metadata) { repository.archive_metadata(ref, storage_path, 'gitlab-git-test', format, append_sha: append_sha) } it 'sets CommitId to the commit SHA' do expect(metadata['CommitId']).to eq(SeedRepo::LastCommit::ID) @@ -373,6 +374,7 @@ describe Gitlab::Git::Repository, seed_helper: true do context '#submodules' do around do |example| + # TODO #submodules will be removed, has been migrated to gitaly Gitlab::GitalyClient::StorageSettings.allow_disk_access do example.run end @@ -1055,6 +1057,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#rugged_commits_between" do around do |example| + # TODO #rugged_commits_between will be removed, has been migrated to gitaly Gitlab::GitalyClient::StorageSettings.allow_disk_access do example.run end @@ -1703,6 +1706,7 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:refs) { ['deadbeef', SeedRepo::RubyBlob::ID, '909e6157199'] } around do |example| + # TODO #batch_existence isn't used anywhere, can we remove it? Gitlab::GitalyClient::StorageSettings.allow_disk_access do example.run end diff --git a/spec/lib/gitlab/git/rev_list_spec.rb b/spec/lib/gitlab/git/rev_list_spec.rb index 32ec1e029c8..95dc47e2a00 100644 --- a/spec/lib/gitlab/git/rev_list_spec.rb +++ b/spec/lib/gitlab/git/rev_list_spec.rb @@ -9,9 +9,11 @@ describe Gitlab::Git::RevList do end def stub_popen_rev_list(*additional_args, with_lazy_block: true, output:) + repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository.path } + params = [ args_for_popen(additional_args), - repository.path, + repo_path, {}, hash_including(lazy_block: with_lazy_block ? anything : nil) ] diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index dfffea7797f..0d5f6a0b576 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -552,7 +552,7 @@ describe Gitlab::GitAccess do it 'returns not found' do project.add_guest(user) repo = project.repository - FileUtils.rm_rf(repo.path) + Gitlab::GitalyClient::StorageSettings.allow_disk_access { FileUtils.rm_rf(repo.path) } # Sanity check for rm_rf expect(repo.exists?).to eq(false) @@ -750,20 +750,22 @@ describe Gitlab::GitAccess do def merge_into_protected_branch @protected_branch_merge_commit ||= begin - stub_git_hooks - project.repository.add_branch(user, unprotected_branch, 'feature') - target_branch = project.repository.lookup('feature') - source_branch = project.repository.create_file( - user, - 'filename', - 'This is the file content', - message: 'This is a good commit message', - branch_name: unprotected_branch) - rugged = project.repository.rugged - author = { email: "email@example.com", time: Time.now, name: "Example Git User" } - - merge_index = rugged.merge_commits(target_branch, source_branch) - Rugged::Commit.create(rugged, author: author, committer: author, message: "commit message", parents: [target_branch, source_branch], tree: merge_index.write_tree(rugged)) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + stub_git_hooks + project.repository.add_branch(user, unprotected_branch, 'feature') + target_branch = project.repository.lookup('feature') + source_branch = project.repository.create_file( + user, + 'filename', + 'This is the file content', + message: 'This is a good commit message', + branch_name: unprotected_branch) + rugged = project.repository.rugged + author = { email: "email@example.com", time: Time.now, name: "Example Git User" } + + merge_index = rugged.merge_commits(target_branch, source_branch) + Rugged::Commit.create(rugged, author: author, committer: author, message: "commit message", parents: [target_branch, source_branch], tree: merge_index.write_tree(rugged)) + end end end diff --git a/spec/lib/gitlab/gpg_spec.rb b/spec/lib/gitlab/gpg_spec.rb index ab9a166db00..47f37cae98f 100644 --- a/spec/lib/gitlab/gpg_spec.rb +++ b/spec/lib/gitlab/gpg_spec.rb @@ -74,6 +74,19 @@ describe Gitlab::Gpg do email: 'nannie.bernhard@example.com' }]) end + + it 'rejects non UTF-8 names and addresses' do + public_key = double(:key) + fingerprints = double(:fingerprints) + email = "\xEEch@test.com".force_encoding('ASCII-8BIT') + uid = double(:uid, name: 'Test User', email: email) + raw_key = double(:raw_key, uids: [uid]) + allow(Gitlab::Gpg::CurrentKeyChain).to receive(:fingerprints_from_key).with(public_key).and_return(fingerprints) + allow(GPGME::Key).to receive(:find).with(:public, anything).and_return([raw_key]) + + user_infos = described_class.user_infos_from_key(public_key) + expect(user_infos).to eq([]) + end end describe '.current_home_dir' do diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index a129855dbd8..2ea66479c1b 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -257,6 +257,7 @@ project: - import_data - commit_statuses - pipelines +- stages - builds - runner_projects - runners diff --git a/spec/lib/gitlab/import_export/fork_spec.rb b/spec/lib/gitlab/import_export/fork_spec.rb index 17e06a6a83f..71fd5a51c3b 100644 --- a/spec/lib/gitlab/import_export/fork_spec.rb +++ b/spec/lib/gitlab/import_export/fork_spec.rb @@ -41,8 +41,10 @@ describe 'forked project import' do after do FileUtils.rm_rf(export_path) - FileUtils.rm_rf(project_with_repo.repository.path_to_repo) - FileUtils.rm_rf(project.repository.path_to_repo) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + FileUtils.rm_rf(project_with_repo.repository.path_to_repo) + FileUtils.rm_rf(project.repository.path_to_repo) + end end it 'can access the MR' do diff --git a/spec/lib/gitlab/import_export/repo_restorer_spec.rb b/spec/lib/gitlab/import_export/repo_restorer_spec.rb index dc806d036ff..013b8895f67 100644 --- a/spec/lib/gitlab/import_export/repo_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/repo_restorer_spec.rb @@ -23,8 +23,10 @@ describe Gitlab::ImportExport::RepoRestorer do after do FileUtils.rm_rf(export_path) - FileUtils.rm_rf(project_with_repo.repository.path_to_repo) - FileUtils.rm_rf(project.repository.path_to_repo) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + FileUtils.rm_rf(project_with_repo.repository.path_to_repo) + FileUtils.rm_rf(project.repository.path_to_repo) + end end it 'restores the repo successfully' do @@ -34,7 +36,9 @@ describe Gitlab::ImportExport::RepoRestorer do it 'has the webhooks' do restorer.restore - expect(Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository)).to exist + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + expect(Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository)).to exist + end end end end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 2aebdc57f7c..5b289ceb3b2 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -170,7 +170,7 @@ MergeRequest: - last_edited_by_id - head_pipeline_id - discussion_locked -- allow_maintainer_to_push +- allow_collaboration MergeRequestDiff: - id - state diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index bf6ee4b0b59..14eae22a2ec 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -405,7 +405,11 @@ describe Gitlab::Shell do describe '#create_repository' do shared_examples '#create_repository' do let(:repository_storage) { 'default' } - let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage].legacy_disk_path } + let(:repository_storage_path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages[repository_storage].legacy_disk_path + end + end let(:repo_name) { 'project/path' } let(:created_path) { File.join(repository_storage_path, repo_name + '.git') } diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb index 97b6069f64d..0469d984a40 100644 --- a/spec/lib/gitlab/user_access_spec.rb +++ b/spec/lib/gitlab/user_access_spec.rb @@ -142,7 +142,7 @@ describe Gitlab::UserAccess do target_project: canonical_project, source_project: project, source_branch: 'awesome-feature', - allow_maintainer_to_push: true + allow_collaboration: true ) end diff --git a/spec/lib/object_storage/direct_upload_spec.rb b/spec/lib/object_storage/direct_upload_spec.rb index 5187821e8f4..e0569218d78 100644 --- a/spec/lib/object_storage/direct_upload_spec.rb +++ b/spec/lib/object_storage/direct_upload_spec.rb @@ -17,6 +17,10 @@ describe ObjectStorage::DirectUpload do let(:direct_upload) { described_class.new(credentials, bucket_name, object_name, has_length: has_length, maximum_size: maximum_size) } + before do + Fog.unmock! + end + describe '#has_length' do context 'is known' do let(:has_length) { true } diff --git a/spec/migrations/change_default_value_for_dsa_key_restriction_spec.rb b/spec/migrations/change_default_value_for_dsa_key_restriction_spec.rb new file mode 100644 index 00000000000..7e61ab9b52e --- /dev/null +++ b/spec/migrations/change_default_value_for_dsa_key_restriction_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20180531220618_change_default_value_for_dsa_key_restriction.rb') + +describe ChangeDefaultValueForDsaKeyRestriction, :migration do + let(:application_settings) { table(:application_settings) } + + before do + application_settings.create! + end + + it 'changes the default value for dsa_key_restriction' do + expect(application_settings.first.dsa_key_restriction).to eq(0) + + migrate! + + application_settings.reset_column_information + new_setting = application_settings.create! + + expect(application_settings.count).to eq(2) + expect(new_setting.dsa_key_restriction).to eq(-1) + end + + it 'changes the existing setting' do + setting = application_settings.last + + expect(setting.dsa_key_restriction).to eq(0) + + migrate! + + expect(application_settings.count).to eq(1) + expect(setting.reload.dsa_key_restriction).to eq(-1) + end +end diff --git a/spec/models/application_setting/term_spec.rb b/spec/models/application_setting/term_spec.rb index 1eddf3c56ff..aa49594f4d1 100644 --- a/spec/models/application_setting/term_spec.rb +++ b/spec/models/application_setting/term_spec.rb @@ -12,4 +12,41 @@ describe ApplicationSetting::Term do expect(described_class.latest).to eq(terms) end end + + describe '#accepted_by_user?' do + let(:user) { create(:user) } + let(:term) { create(:term) } + + it 'is true when the user accepted the terms' do + accept_terms(term, user) + + expect(term.accepted_by_user?(user)).to be(true) + end + + it 'is false when the user declined the terms' do + decline_terms(term, user) + + expect(term.accepted_by_user?(user)).to be(false) + end + + it 'does not cause a query when the user accepted the current terms' do + accept_terms(term, user) + + expect { term.accepted_by_user?(user) }.not_to exceed_query_limit(0) + end + + it 'returns false if the currently accepted terms are different' do + accept_terms(create(:term), user) + + expect(term.accepted_by_user?(user)).to be(false) + end + + def accept_terms(term, user) + Users::RespondToTermsService.new(user, term).execute(accepted: true) + end + + def decline_terms(term, user) + Users::RespondToTermsService.new(user, term).execute(accepted: false) + end + end end diff --git a/spec/models/ci/group_spec.rb b/spec/models/ci/group_spec.rb index 51123e73fe6..838fa63cb1f 100644 --- a/spec/models/ci/group_spec.rb +++ b/spec/models/ci/group_spec.rb @@ -41,4 +41,55 @@ describe Ci::Group do end end end + + describe '.fabricate' do + let(:pipeline) { create(:ci_empty_pipeline) } + let(:stage) { create(:ci_stage_entity, pipeline: pipeline) } + + before do + create_build(:ci_build, name: 'rspec 0 2') + create_build(:ci_build, name: 'rspec 0 1') + create_build(:ci_build, name: 'spinach 0 1') + create_build(:commit_status, name: 'aaaaa') + end + + it 'returns an array of three groups' do + expect(stage.groups).to be_a Array + expect(stage.groups).to all(be_a described_class) + expect(stage.groups.size).to eq 3 + end + + it 'returns groups with correctly ordered statuses' do + expect(stage.groups.first.jobs.map(&:name)) + .to eq ['aaaaa'] + expect(stage.groups.second.jobs.map(&:name)) + .to eq ['rspec 0 1', 'rspec 0 2'] + expect(stage.groups.third.jobs.map(&:name)) + .to eq ['spinach 0 1'] + end + + it 'returns groups with correct names' do + expect(stage.groups.map(&:name)) + .to eq %w[aaaaa rspec spinach] + end + + context 'when a name is nil on legacy pipelines' do + before do + pipeline.builds.first.update_attribute(:name, nil) + end + + it 'returns an array of three groups' do + expect(stage.groups.map(&:name)) + .to eq ['', 'aaaaa', 'rspec', 'spinach'] + end + end + + def create_build(type, status: 'success', **opts) + create(type, pipeline: pipeline, + stage: stage.name, + status: status, + stage_id: stage.id, + **opts) + end + end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 24692ebb9a3..2bae98dcbb8 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -537,6 +537,87 @@ describe Ci::Pipeline, :mailer do end end end + + describe '#stages' do + before do + create(:ci_stage_entity, project: project, + pipeline: pipeline, + name: 'build') + end + + it 'returns persisted stages' do + expect(pipeline.stages).not_to be_empty + expect(pipeline.stages).to all(be_persisted) + end + end + + describe '#ordered_stages' do + before do + create(:ci_stage_entity, project: project, + pipeline: pipeline, + position: 4, + name: 'deploy') + + create(:ci_build, project: project, + pipeline: pipeline, + stage: 'test', + stage_idx: 3, + name: 'test') + + create(:ci_build, project: project, + pipeline: pipeline, + stage: 'build', + stage_idx: 2, + name: 'build') + + create(:ci_stage_entity, project: project, + pipeline: pipeline, + position: 1, + name: 'sanity') + + create(:ci_stage_entity, project: project, + pipeline: pipeline, + position: 5, + name: 'cleanup') + end + + subject { pipeline.ordered_stages } + + context 'when using legacy stages' do + before do + stub_feature_flags(ci_pipeline_persisted_stages: false) + end + + it 'returns legacy stages in valid order' do + expect(subject.map(&:name)).to eq %w[build test] + end + end + + context 'when using persisted stages' do + before do + stub_feature_flags(ci_pipeline_persisted_stages: true) + end + + context 'when pipelines is not complete' do + it 'still returns legacy stages' do + expect(subject).to all(be_a Ci::LegacyStage) + expect(subject.map(&:name)).to eq %w[build test] + end + end + + context 'when pipeline is complete' do + before do + pipeline.succeed! + end + + it 'returns stages in valid order' do + expect(subject).to all(be_a Ci::Stage) + expect(subject.map(&:name)) + .to eq %w[sanity build test deploy cleanup] + end + end + end + end end describe 'state machine' do @@ -1181,6 +1262,43 @@ describe Ci::Pipeline, :mailer do end end + describe '#update_status' do + context 'when pipeline is empty' do + it 'updates does not change pipeline status' do + expect(pipeline.statuses.latest.status).to be_nil + + expect { pipeline.update_status } + .to change { pipeline.reload.status }.to 'skipped' + end + end + + context 'when updating status to pending' do + before do + allow(pipeline) + .to receive_message_chain(:statuses, :latest, :status) + .and_return(:running) + end + + it 'updates pipeline status to running' do + expect { pipeline.update_status } + .to change { pipeline.reload.status }.to 'running' + end + end + + context 'when statuses status was not recognized' do + before do + allow(pipeline) + .to receive(:latest_builds_status) + .and_return(:unknown) + end + + it 'raises an exception' do + expect { pipeline.update_status } + .to raise_error(HasStatus::UnknownStatusError) + end + end + end + describe '#detailed_status' do subject { pipeline.detailed_status(user) } diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 0f072aa1719..f6433234573 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -549,7 +549,7 @@ describe Ci::Runner do end describe '#update_cached_info' do - let(:runner) { create(:ci_runner) } + let(:runner) { create(:ci_runner, :project) } subject { runner.update_cached_info(architecture: '18-bit') } @@ -570,17 +570,22 @@ describe Ci::Runner do runner.contacted_at = 2.hours.ago end - it 'updates database' do - expect_redis_update + context 'with invalid runner' do + before do + runner.projects = [] + end + + it 'still updates redis cache and database' do + expect(runner).to be_invalid - expect { subject }.to change { runner.reload.read_attribute(:contacted_at) } - .and change { runner.reload.read_attribute(:architecture) } + expect_redis_update + does_db_update + end end - it 'updates cache' do + it 'updates redis cache and database' do expect_redis_update - - subject + does_db_update end end @@ -590,6 +595,11 @@ describe Ci::Runner do expect(redis).to receive(:set).with(redis_key, anything, any_args) end end + + def does_db_update + expect { subject }.to change { runner.reload.read_attribute(:contacted_at) } + .and change { runner.reload.read_attribute(:architecture) } + end end describe '#destroy' do diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index a00db1d2bfc..22a4556c10c 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -65,7 +65,31 @@ describe Ci::Stage, :models do end end - context 'when stage is skipped' do + context 'when stage has only created builds' do + let(:stage) { create(:ci_stage_entity, status: :created) } + + before do + create(:ci_build, :created, stage_id: stage.id) + end + + it 'updates status to skipped' do + expect(stage.reload.status).to eq 'created' + end + end + + context 'when stage is skipped because of skipped builds' do + before do + create(:ci_build, :skipped, stage_id: stage.id) + end + + it 'updates status to skipped' do + expect { stage.update_status } + .to change { stage.reload.status } + .to 'skipped' + end + end + + context 'when stage is skipped because is empty' do it 'updates status to skipped' do expect { stage.update_status } .to change { stage.reload.status } @@ -86,9 +110,85 @@ describe Ci::Stage, :models do expect(stage.reload).to be_failed end end + + context 'when statuses status was not recognized' do + before do + allow(stage) + .to receive_message_chain(:statuses, :latest, :status) + .and_return(:unknown) + end + + it 'raises an exception' do + expect { stage.update_status } + .to raise_error(HasStatus::UnknownStatusError) + end + end + end + + describe '#detailed_status' do + using RSpec::Parameterized::TableSyntax + + let(:user) { create(:user) } + let(:stage) { create(:ci_stage_entity, status: :created) } + subject { stage.detailed_status(user) } + + where(:statuses, :label) do + %w[created] | :created + %w[success] | :passed + %w[pending] | :pending + %w[skipped] | :skipped + %w[canceled] | :canceled + %w[success failed] | :failed + %w[running pending] | :running + end + + with_them do + before do + statuses.each do |status| + create(:commit_status, project: stage.project, + pipeline: stage.pipeline, + stage_id: stage.id, + status: status) + + stage.update_status + end + end + + it 'has a correct label' do + expect(subject.label).to eq label.to_s + end + end + + context 'when stage has warnings' do + before do + create(:ci_build, project: stage.project, + pipeline: stage.pipeline, + stage_id: stage.id, + status: :failed, + allow_failure: true) + + stage.update_status + end + + it 'is passed with warnings' do + expect(subject.label).to eq 'passed with warnings' + end + end end - describe '#index' do + describe '#groups' do + before do + create(:ci_build, stage_id: stage.id, name: 'rspec 0 1') + create(:ci_build, stage_id: stage.id, name: 'rspec 0 2') + end + + it 'groups stage builds by name' do + expect(stage.groups).to be_one + expect(stage.groups.first.name).to eq 'rspec' + end + end + + describe '#position' do context 'when stage has been imported and does not have position index set' do before do stage.update_column(:position, nil) @@ -119,4 +219,42 @@ describe Ci::Stage, :models do end end end + + context 'when stage has warnings' do + before do + create(:ci_build, :failed, :allowed_to_fail, stage_id: stage.id) + end + + describe '#has_warnings?' do + it 'returns true' do + expect(stage).to have_warnings + end + end + + describe '#number_of_warnings' do + it 'returns a lazy stage warnings counter' do + lazy_queries = ActiveRecord::QueryRecorder.new do + stage.number_of_warnings + end + + synced_queries = ActiveRecord::QueryRecorder.new do + stage.number_of_warnings.to_i + end + + expect(lazy_queries.count).to eq 0 + expect(synced_queries.count).to eq 1 + + expect(stage.number_of_warnings.inspect).to include 'BatchLoader' + expect(stage.number_of_warnings).to eq 1 + end + end + end + + context 'when stage does not have warnings' do + describe '#has_warnings?' do + it 'returns false' do + expect(stage).not_to have_warnings + end + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 65cc9372cbe..3f028b3bd5c 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -16,7 +16,11 @@ describe MergeRequest do describe '#squash_in_progress?' do shared_examples 'checking whether a squash is in progress' do - let(:repo_path) { subject.source_project.repository.path } + let(:repo_path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + subject.source_project.repository.path + end + end let(:squash_path) { File.join(repo_path, "gitlab-worktree", "squash-#{subject.id}") } before do @@ -2197,7 +2201,11 @@ describe MergeRequest do describe '#rebase_in_progress?' do shared_examples 'checking whether a rebase is in progress' do - let(:repo_path) { subject.source_project.repository.path } + let(:repo_path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + subject.source_project.repository.path + end + end let(:rebase_path) { File.join(repo_path, "gitlab-worktree", "rebase-#{subject.id}") } before do @@ -2237,25 +2245,25 @@ describe MergeRequest do end end - describe '#allow_maintainer_to_push' do + describe '#allow_collaboration' do let(:merge_request) do - build(:merge_request, source_branch: 'fixes', allow_maintainer_to_push: true) + build(:merge_request, source_branch: 'fixes', allow_collaboration: true) end it 'is false when pushing by a maintainer is not possible' do - expect(merge_request).to receive(:maintainer_push_possible?) { false } + expect(merge_request).to receive(:collaborative_push_possible?) { false } - expect(merge_request.allow_maintainer_to_push).to be_falsy + expect(merge_request.allow_collaboration).to be_falsy end it 'is true when pushing by a maintainer is possible' do - expect(merge_request).to receive(:maintainer_push_possible?) { true } + expect(merge_request).to receive(:collaborative_push_possible?) { true } - expect(merge_request.allow_maintainer_to_push).to be_truthy + expect(merge_request.allow_collaboration).to be_truthy end end - describe '#maintainer_push_possible?' do + describe '#collaborative_push_possible?' do let(:merge_request) do build(:merge_request, source_branch: 'fixes') end @@ -2267,14 +2275,14 @@ describe MergeRequest do it 'does not allow maintainer to push if the source project is the same as the target' do merge_request.target_project = merge_request.source_project = create(:project, :public) - expect(merge_request.maintainer_push_possible?).to be_falsy + expect(merge_request.collaborative_push_possible?).to be_falsy end it 'allows maintainer to push when both source and target are public' do merge_request.target_project = build(:project, :public) merge_request.source_project = build(:project, :public) - expect(merge_request.maintainer_push_possible?).to be_truthy + expect(merge_request.collaborative_push_possible?).to be_truthy end it 'is not available for protected branches' do @@ -2285,11 +2293,11 @@ describe MergeRequest do .with(merge_request.source_project, 'fixes') .and_return(true) - expect(merge_request.maintainer_push_possible?).to be_falsy + expect(merge_request.collaborative_push_possible?).to be_falsy end end - describe '#can_allow_maintainer_to_push?' do + describe '#can_allow_collaboration?' do let(:target_project) { create(:project, :public) } let(:source_project) { fork_project(target_project) } let(:merge_request) do @@ -2301,17 +2309,17 @@ describe MergeRequest do let(:user) { create(:user) } before do - allow(merge_request).to receive(:maintainer_push_possible?) { true } + allow(merge_request).to receive(:collaborative_push_possible?) { true } end it 'is false if the user does not have push access to the source project' do - expect(merge_request.can_allow_maintainer_to_push?(user)).to be_falsy + expect(merge_request.can_allow_collaboration?(user)).to be_falsy end it 'is true when the user has push access to the source project' do source_project.add_developer(user) - expect(merge_request.can_allow_maintainer_to_push?(user)).to be_truthy + expect(merge_request.can_allow_collaboration?(user)).to be_truthy end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 6f702d8d95e..18b01c3e6b7 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -301,12 +301,18 @@ describe Namespace do end def project_rugged(project) - project.repository.rugged + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.rugged + end end end describe '#rm_dir', 'callback' do - let(:repository_storage_path) { Gitlab.config.repositories.storages.default.legacy_disk_path } + let(:repository_storage_path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages.default.legacy_disk_path + end + end let(:path_in_dir) { File.join(repository_storage_path, namespace.full_path) } let(:deleted_path) { namespace.full_path.gsub(namespace.path, "#{namespace.full_path}+#{namespace.id}+deleted") } let(:deleted_path_in_dir) { File.join(repository_storage_path, deleted_path) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9a76452a808..fe9d64c0e3b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3583,7 +3583,7 @@ describe Project do target_branch: 'target-branch', source_project: project, source_branch: 'awesome-feature-1', - allow_maintainer_to_push: true + allow_collaboration: true ) end @@ -3620,9 +3620,9 @@ describe Project do end end - describe '#branch_allows_maintainer_push?' do + describe '#branch_allows_collaboration_push?' do it 'allows access if the user can merge the merge request' do - expect(project.branch_allows_maintainer_push?(user, 'awesome-feature-1')) + expect(project.branch_allows_collaboration?(user, 'awesome-feature-1')) .to be_truthy end @@ -3630,7 +3630,7 @@ describe Project do guest = create(:user) target_project.add_guest(guest) - expect(project.branch_allows_maintainer_push?(guest, 'awesome-feature-1')) + expect(project.branch_allows_collaboration?(guest, 'awesome-feature-1')) .to be_falsy end @@ -3640,31 +3640,31 @@ describe Project do target_branch: 'target-branch', source_project: project, source_branch: 'rejected-feature-1', - allow_maintainer_to_push: true) + allow_collaboration: true) - expect(project.branch_allows_maintainer_push?(user, 'rejected-feature-1')) + expect(project.branch_allows_collaboration?(user, 'rejected-feature-1')) .to be_falsy end it 'does not allow access if the user cannot merge the merge request' do create(:protected_branch, :masters_can_push, project: target_project, name: 'target-branch') - expect(project.branch_allows_maintainer_push?(user, 'awesome-feature-1')) + expect(project.branch_allows_collaboration?(user, 'awesome-feature-1')) .to be_falsy end it 'caches the result' do - control = ActiveRecord::QueryRecorder.new { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } + control = ActiveRecord::QueryRecorder.new { project.branch_allows_collaboration?(user, 'awesome-feature-1') } - expect { 3.times { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } } + expect { 3.times { project.branch_allows_collaboration?(user, 'awesome-feature-1') } } .not_to exceed_query_limit(control) end context 'when the requeststore is active', :request_store do it 'only queries per project across instances' do - control = ActiveRecord::QueryRecorder.new { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } + control = ActiveRecord::QueryRecorder.new { project.branch_allows_collaboration?(user, 'awesome-feature-1') } - expect { 2.times { described_class.find(project.id).branch_allows_maintainer_push?(user, 'awesome-feature-1') } } + expect { 2.times { described_class.find(project.id).branch_allows_collaboration?(user, 'awesome-feature-1') } } .not_to exceed_query_limit(control).with_threshold(2) end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 0ccf55bd895..7c0a1cd967c 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -136,7 +136,10 @@ describe Repository do before do options = { message: 'test tag message\n', tagger: { name: 'John Smith', email: 'john@gmail.com' } } - repository.rugged.tags.create(annotated_tag_name, 'a48e4fc218069f68ef2e769dd8dfea3991362175', options) + + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.rugged.tags.create(annotated_tag_name, 'a48e4fc218069f68ef2e769dd8dfea3991362175', options) + end double_first = double(committed_date: Time.now - 1.second) double_last = double(committed_date: Time.now) @@ -1048,6 +1051,13 @@ describe Repository do let(:target_project) { project } let(:target_repository) { target_project.repository } + around do |example| + # TODO Gitlab::Git::OperationService will be moved to gitaly-ruby and disappear from this repo + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + context 'when pre hooks were successful' do before do service = Gitlab::Git::HooksService.new @@ -1309,6 +1319,13 @@ describe Repository do end describe '#update_autocrlf_option' do + around do |example| + # TODO Gitlab::Git::OperationService will be moved to gitaly-ruby and disappear from this repo + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + describe 'when autocrlf is not already set to :input' do before do repository.raw_repository.autocrlf = true @@ -1802,7 +1819,9 @@ describe Repository do expect(repository.branch_count).to be_an(Integer) # NOTE: Until rugged goes away, make sure rugged and gitaly are in sync - rugged_count = repository.raw_repository.rugged.branches.count + rugged_count = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.raw_repository.rugged.branches.count + end expect(repository.branch_count).to eq(rugged_count) end @@ -1813,7 +1832,9 @@ describe Repository do expect(repository.tag_count).to be_an(Integer) # NOTE: Until rugged goes away, make sure rugged and gitaly are in sync - rugged_count = repository.raw_repository.rugged.tags.count + rugged_count = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.raw_repository.rugged.tags.count + end expect(repository.tag_count).to eq(rugged_count) end @@ -2073,7 +2094,10 @@ describe Repository do it "attempting to call keep_around on truncated ref does not fail" do repository.keep_around(sample_commit.id) ref = repository.send(:keep_around_ref_name, sample_commit.id) - path = File.join(repository.path, ref) + + path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join(repository.path, ref) + end # Corrupt the reference File.truncate(path, 0) @@ -2088,6 +2112,13 @@ describe Repository do end describe '#update_ref' do + around do |example| + # TODO Gitlab::Git::OperationService will be moved to gitaly-ruby and disappear from this repo + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + it 'can create a ref' do Gitlab::Git::OperationService.new(nil, repository.raw_repository).send(:update_ref, 'refs/heads/foobar', 'refs/heads/master', Gitlab::Git::BLANK_SHA) @@ -2372,7 +2403,9 @@ describe Repository do end def create_remote_branch(remote_name, branch_name, target) - rugged = repository.rugged + rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.rugged + end rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target.id) end @@ -2410,6 +2443,32 @@ describe Repository do end end + describe '#archive_metadata' do + let(:ref) { 'master' } + let(:storage_path) { '/tmp' } + + let(:prefix) { [project.path, ref].join('-') } + let(:filename) { prefix + '.tar.gz' } + + subject(:result) { repository.archive_metadata(ref, storage_path, append_sha: false) } + + context 'with hashed storage disabled' do + let(:project) { create(:project, :repository, :legacy_storage) } + + it 'uses the project path to generate the filename' do + expect(result['ArchivePrefix']).to eq(prefix) + expect(File.basename(result['ArchivePath'])).to eq(filename) + end + end + + context 'with hashed storage enabled' do + it 'uses the project path to generate the filename' do + expect(result['ArchivePrefix']).to eq(prefix) + expect(File.basename(result['ArchivePath'])).to eq(filename) + end + end + end + describe 'commit cache' do set(:project) { create(:project, :repository) } diff --git a/spec/models/term_agreement_spec.rb b/spec/models/term_agreement_spec.rb index a59bf119692..950dfa09a6a 100644 --- a/spec/models/term_agreement_spec.rb +++ b/spec/models/term_agreement_spec.rb @@ -5,4 +5,13 @@ describe TermAgreement do it { is_expected.to validate_presence_of(:term) } it { is_expected.to validate_presence_of(:user) } end + + describe '.accepted' do + it 'only includes accepted terms' do + accepted = create(:term_agreement, :accepted) + create(:term_agreement, :declined) + + expect(described_class.accepted).to contain_exactly(accepted) + end + end end diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb index 9ca156deaa0..eead55d33ca 100644 --- a/spec/policies/ci/build_policy_spec.rb +++ b/spec/policies/ci/build_policy_spec.rb @@ -101,7 +101,7 @@ describe Ci::BuildPolicy do it 'enables update_build if user is maintainer' do allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false) - allow_any_instance_of(Project).to receive(:branch_allows_maintainer_push?).and_return(true) + allow_any_instance_of(Project).to receive(:branch_allows_collaboration?).and_return(true) expect(policy).to be_allowed :update_build expect(policy).to be_allowed :update_commit_status diff --git a/spec/policies/ci/pipeline_policy_spec.rb b/spec/policies/ci/pipeline_policy_spec.rb index a5e509cfa0f..bd32faf06ef 100644 --- a/spec/policies/ci/pipeline_policy_spec.rb +++ b/spec/policies/ci/pipeline_policy_spec.rb @@ -69,7 +69,7 @@ describe Ci::PipelinePolicy, :models do it 'enables update_pipeline if user is maintainer' do allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false) - allow_any_instance_of(Project).to receive(:branch_allows_maintainer_push?).and_return(true) + allow_any_instance_of(Project).to receive(:branch_allows_collaboration?).and_return(true) expect(policy).to be_allowed :update_pipeline end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 6609f5f7afd..6ac151f92f3 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -400,7 +400,7 @@ describe ProjectPolicy do :merge_request, target_project: target_project, source_project: project, - allow_maintainer_to_push: true + allow_collaboration: true ) end let(:maintainer_abilities) do diff --git a/spec/requests/api/events_spec.rb b/spec/requests/api/events_spec.rb index 962c845f36d..e6a61fdcf39 100644 --- a/spec/requests/api/events_spec.rb +++ b/spec/requests/api/events_spec.rb @@ -176,7 +176,7 @@ describe API::Events do end it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do get api("/projects/#{private_project.id}/events", user), target_type: :merge_request end.count @@ -184,7 +184,7 @@ describe API::Events do expect do get api("/projects/#{private_project.id}/events", user), target_type: :merge_request - end.not_to exceed_query_limit(control_count) + end.not_to exceed_all_query_limit(control_count) expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 5dc3ddd4b36..bc32372d3a9 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -835,8 +835,7 @@ describe API::Internal do end def push(key, project, protocol = 'ssh', env: nil) - post( - api("/internal/allowed"), + params = { changes: 'd14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master', key_id: key.id, project: project.full_path, @@ -845,7 +844,19 @@ describe API::Internal do secret_token: secret_token, protocol: protocol, env: env - ) + } + + if Gitlab.rails5? + post( + api("/internal/allowed"), + params: params + ) + else + post( + api("/internal/allowed"), + params + ) + end end def archive(key, project) diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 605761867bf..d4ebfc3f782 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -386,12 +386,13 @@ describe API::MergeRequests do source_project: forked_project, target_project: project, source_branch: 'fixes', - allow_maintainer_to_push: true) + allow_collaboration: true) end - it 'includes the `allow_maintainer_to_push` field' do + it 'includes the `allow_collaboration` field' do get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) + expect(json_response['allow_collaboration']).to be_truthy expect(json_response['allow_maintainer_to_push']).to be_truthy end end @@ -654,11 +655,12 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(400) end - it 'allows setting `allow_maintainer_to_push`' do + it 'allows setting `allow_collaboration`' do post api("/projects/#{forked_project.id}/merge_requests", user2), - title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master", - author: user2, target_project_id: project.id, allow_maintainer_to_push: true + title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master", + author: user2, target_project_id: project.id, allow_collaboration: true expect(response).to have_gitlab_http_status(201) + expect(json_response['allow_collaboration']).to be_truthy expect(json_response['allow_maintainer_to_push']).to be_truthy end diff --git a/spec/requests/api/pipelines_spec.rb b/spec/requests/api/pipelines_spec.rb index 0736329f9fd..78ea77cb3bb 100644 --- a/spec/requests/api/pipelines_spec.rb +++ b/spec/requests/api/pipelines_spec.rb @@ -285,6 +285,15 @@ describe API::Pipelines do end describe 'POST /projects/:id/pipeline ' do + def expect_variables(variables, expected_variables) + variables.each_with_index do |variable, index| + expected_variable = expected_variables[index] + + expect(variable.key).to eq(expected_variable['key']) + expect(variable.value).to eq(expected_variable['value']) + end + end + context 'authorized user' do context 'with gitlab-ci.yml' do before do @@ -294,13 +303,62 @@ describe API::Pipelines do it 'creates and returns a new pipeline' do expect do post api("/projects/#{project.id}/pipeline", user), ref: project.default_branch - end.to change { Ci::Pipeline.count }.by(1) + end.to change { project.pipelines.count }.by(1) expect(response).to have_gitlab_http_status(201) expect(json_response).to be_a Hash expect(json_response['sha']).to eq project.commit.id end + context 'variables given' do + let(:variables) { [{ 'key' => 'UPLOAD_TO_S3', 'value' => 'true' }] } + + it 'creates and returns a new pipeline using the given variables' do + expect do + post api("/projects/#{project.id}/pipeline", user), ref: project.default_branch, variables: variables + end.to change { project.pipelines.count }.by(1) + expect_variables(project.pipelines.last.variables, variables) + + expect(response).to have_gitlab_http_status(201) + expect(json_response).to be_a Hash + expect(json_response['sha']).to eq project.commit.id + expect(json_response).not_to have_key('variables') + end + end + + describe 'using variables conditions' do + let(:variables) { [{ 'key' => 'STAGING', 'value' => 'true' }] } + + before do + config = YAML.dump(test: { script: 'test', only: { variables: ['$STAGING'] } }) + stub_ci_pipeline_yaml_file(config) + end + + it 'creates and returns a new pipeline using the given variables' do + expect do + post api("/projects/#{project.id}/pipeline", user), ref: project.default_branch, variables: variables + end.to change { project.pipelines.count }.by(1) + expect_variables(project.pipelines.last.variables, variables) + + expect(response).to have_gitlab_http_status(201) + expect(json_response).to be_a Hash + expect(json_response['sha']).to eq project.commit.id + expect(json_response).not_to have_key('variables') + end + + context 'condition unmatch' do + let(:variables) { [{ 'key' => 'STAGING', 'value' => 'false' }] } + + it "doesn't create a job" do + expect do + post api("/projects/#{project.id}/pipeline", user), ref: project.default_branch + end.not_to change { project.pipelines.count } + + expect(response).to have_gitlab_http_status(400) + end + end + end + it 'fails when using an invalid ref' do post api("/projects/#{project.id}/pipeline", user), ref: 'invalid_ref' diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index 9e6d69e3874..cd135dfc32a 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -220,11 +220,10 @@ describe API::Repositories do expect(response).to have_gitlab_http_status(200) - repo_name = project.repository.name.gsub("\.git", "") type, params = workhorse_send_data expect(type).to eq('git-archive') - expect(params['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.tar.gz/) + expect(params['ArchivePath']).to match(/#{project.path}\-[^\.]+\.tar.gz/) end it 'returns the repository archive archive.zip' do @@ -232,11 +231,10 @@ describe API::Repositories do expect(response).to have_gitlab_http_status(200) - repo_name = project.repository.name.gsub("\.git", "") type, params = workhorse_send_data expect(type).to eq('git-archive') - expect(params['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.zip/) + expect(params['ArchivePath']).to match(/#{project.path}\-[^\.]+\.zip/) end it 'returns the repository archive archive.tar.bz2' do @@ -244,11 +242,10 @@ describe API::Repositories do expect(response).to have_gitlab_http_status(200) - repo_name = project.repository.name.gsub("\.git", "") type, params = workhorse_send_data expect(type).to eq('git-archive') - expect(params['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.tar.bz2/) + expect(params['ArchivePath']).to match(/#{project.path}\-[^\.]+\.tar.bz2/) end context 'when sha does not exist' do diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index e2b19ad59f9..969710d6613 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -287,7 +287,10 @@ describe API::Tags do context 'annotated tag' do it 'creates a new annotated tag' do # Identity must be set in .gitconfig to create annotated tag. - repo_path = project.repository.path_to_repo + repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.path_to_repo + end + system(*%W(#{Gitlab.config.git.bin_path} --git-dir=#{repo_path} config user.name #{user.name})) system(*%W(#{Gitlab.config.git.bin_path} --git-dir=#{repo_path} config user.email #{user.email})) diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index b741308e2c5..e0e6eecb300 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -8,6 +8,10 @@ describe PipelineSerializer do described_class.new(current_user: user) end + before do + stub_feature_flags(ci_pipeline_persisted_stages: true) + end + subject { serializer.represent(resource) } describe '#represent' do @@ -99,7 +103,8 @@ describe PipelineSerializer do end end - context 'number of queries' do + describe 'number of queries when preloaded' do + subject { serializer.represent(resource, preload: true) } let(:resource) { Ci::Pipeline.all } before do @@ -107,7 +112,7 @@ describe PipelineSerializer do # gitaly calls in this block # Issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/37772 Gitlab::GitalyClient.allow_n_plus_1_calls do - Ci::Pipeline::AVAILABLE_STATUSES.each do |status| + Ci::Pipeline::COMPLETED_STATUSES.each do |status| create_pipeline(status) end end @@ -120,7 +125,7 @@ describe PipelineSerializer do it 'verifies number of queries', :request_store do recorded = ActiveRecord::QueryRecorder.new { subject } - expect(recorded.count).to be_within(1).of(44) + expect(recorded.count).to be_within(2).of(27) expect(recorded.cached_count).to eq(0) end end @@ -139,7 +144,7 @@ describe PipelineSerializer do # pipeline. With the same ref this check is cached but if refs are # different then there is an extra query per ref # https://gitlab.com/gitlab-org/gitlab-ce/issues/46368 - expect(recorded.count).to be_within(1).of(51) + expect(recorded.count).to be_within(2).of(30) expect(recorded.cached_count).to eq(0) end end diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index a73bd7a0268..688d3b8c038 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -280,12 +280,12 @@ describe Ci::RetryPipelineService, '#execute' do source_project: forked_project, target_project: project, source_branch: 'fixes', - allow_maintainer_to_push: true) + allow_collaboration: true) create_build('rspec 1', :failed, 1) end it 'allows to retry failed pipeline' do - allow_any_instance_of(Project).to receive(:fetch_branch_allows_maintainer_push?).and_return(true) + allow_any_instance_of(Project).to receive(:fetch_branch_allows_collaboration?).and_return(true) allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false) service.execute(pipeline) diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index bd2e91f1f7a..443dcd92a8b 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -547,7 +547,7 @@ describe MergeRequests::UpdateService, :mailer do let(:closed_issuable) { create(:closed_merge_request, source_project: project) } end - context 'setting `allow_maintainer_to_push`' do + context 'setting `allow_collaboration`' do let(:target_project) { create(:project, :public) } let(:source_project) { fork_project(target_project) } let(:user) { create(:user) } @@ -562,23 +562,23 @@ describe MergeRequests::UpdateService, :mailer do allow(ProtectedBranch).to receive(:protected?).with(source_project, 'fixes') { false } end - it 'does not allow a maintainer of the target project to set `allow_maintainer_to_push`' do + it 'does not allow a maintainer of the target project to set `allow_collaboration`' do target_project.add_developer(user) - update_merge_request(allow_maintainer_to_push: true, title: 'Updated title') + update_merge_request(allow_collaboration: true, title: 'Updated title') expect(merge_request.title).to eq('Updated title') - expect(merge_request.allow_maintainer_to_push).to be_falsy + expect(merge_request.allow_collaboration).to be_falsy end it 'is allowed by a user that can push to the source and can update the merge request' do merge_request.update!(assignee: user) source_project.add_developer(user) - update_merge_request(allow_maintainer_to_push: true, title: 'Updated title') + update_merge_request(allow_collaboration: true, title: 'Updated title') expect(merge_request.title).to eq('Updated title') - expect(merge_request.allow_maintainer_to_push).to be_truthy + expect(merge_request.allow_collaboration).to be_truthy end end end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 3e6483d7e28..5100987c2fe 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -64,7 +64,7 @@ describe Projects::TransferService do it 'updates project full path in .git/config' do transfer_project(project, user, group) - expect(project.repository.rugged.config['gitlab.fullpath']).to eq "#{group.full_path}/#{project.path}" + expect(rugged_config['gitlab.fullpath']).to eq "#{group.full_path}/#{project.path}" end end @@ -84,7 +84,9 @@ describe Projects::TransferService do end def project_path(project) - project.repository.path_to_repo + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.path_to_repo + end end def current_path @@ -101,7 +103,7 @@ describe Projects::TransferService do it 'rolls back project full path in .git/config' do attempt_project_transfer - expect(project.repository.rugged.config['gitlab.fullpath']).to eq project.full_path + expect(rugged_config['gitlab.fullpath']).to eq project.full_path end it "doesn't send move notifications" do @@ -264,4 +266,10 @@ describe Projects::TransferService do transfer_project(project, owner, group) end end + + def rugged_config + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.rugged.config + end + end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 1f761bcbbad..ecf1ba05618 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -125,7 +125,7 @@ describe Projects::UpdateService do context 'when we update project but not enabling a wiki' do it 'does not try to create an empty wiki' do - FileUtils.rm_rf(project.wiki.repository.path) + Gitlab::Shell.new.rm_directory(project.repository_storage, project.wiki.path) result = update_project(project, user, { name: 'test1' }) @@ -146,7 +146,7 @@ describe Projects::UpdateService do context 'when enabling a wiki' do it 'creates a wiki' do project.project_feature.update(wiki_access_level: ProjectFeature::DISABLED) - FileUtils.rm_rf(project.wiki.repository.path) + Gitlab::Shell.new.rm_directory(project.repository_storage, project.wiki.path) result = update_project(project, user, project_feature_attributes: { wiki_access_level: ProjectFeature::ENABLED }) diff --git a/spec/services/test_hooks/project_service_spec.rb b/spec/services/test_hooks/project_service_spec.rb index 962b9f40c4f..19e1c5ff3b2 100644 --- a/spec/services/test_hooks/project_service_spec.rb +++ b/spec/services/test_hooks/project_service_spec.rb @@ -6,13 +6,19 @@ describe TestHooks::ProjectService do describe '#execute' do let(:project) { create(:project, :repository) } let(:hook) { create(:project_hook, project: project) } + let(:trigger) { 'not_implemented_events' } let(:service) { described_class.new(hook, current_user, trigger) } let(:sample_data) { { data: 'sample' } } let(:success_result) { { status: :success, http_status: 200, message: 'ok' } } - context 'hook with not implemented test' do - let(:trigger) { 'not_implemented_events' } + it 'allows to set a custom project' do + project = double + service.project = project + + expect(service.project).to eq(project) + end + context 'hook with not implemented test' do it 'returns error message' do expect(hook).not_to receive(:execute) expect(service.execute).to include({ status: :error, message: 'Testing not available for this hook' }) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d3de2331244..e093444121a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -133,6 +133,10 @@ RSpec.configure do |config| RequestStore.clear! end + config.after(:example) do + Fog.unmock! if Fog.mock? + end + config.before(:example, :mailer) do reset_delivered_emails! end diff --git a/spec/support/helpers/cycle_analytics_helpers.rb b/spec/support/helpers/cycle_analytics_helpers.rb index 06a76d53354..32d9807f06a 100644 --- a/spec/support/helpers/cycle_analytics_helpers.rb +++ b/spec/support/helpers/cycle_analytics_helpers.rb @@ -123,7 +123,11 @@ module CycleAnalyticsHelpers if branch_update.newrev _, opts = args - commit = raw_repository.commit(branch_update.newrev).rugged_commit + + commit = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + raw_repository.commit(branch_update.newrev).rugged_commit + end + branch_update.newrev = commit.amend( update_ref: "#{Gitlab::Git::BRANCH_REF_PREFIX}#{opts[:branch_name]}", author: commit.author.merge(time: new_date), diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index c9252bebb2e..93a436cb2b5 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -101,7 +101,9 @@ describe 'gitlab:app namespace rake task' do before do stub_env('SKIP', 'db') - path = File.join(project.repository.path_to_repo, 'custom_hooks') + path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + File.join(project.repository.path_to_repo, 'custom_hooks') + end FileUtils.mkdir_p(path) FileUtils.touch(File.join(path, "dummy.txt")) end @@ -122,7 +124,10 @@ describe 'gitlab:app namespace rake task' do expect { run_rake_task('gitlab:backup:create') }.to output.to_stdout expect { run_rake_task('gitlab:backup:restore') }.to output.to_stdout - expect(Dir.entries(File.join(project.repository.path, 'custom_hooks'))).to include("dummy.txt") + repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.path + end + expect(Dir.entries(File.join(repo_path, 'custom_hooks'))).to include("dummy.txt") end end @@ -243,10 +248,12 @@ describe 'gitlab:app namespace rake task' do FileUtils.mkdir_p(b_storage_dir) # Even when overriding the storage, we have to move it there, so it exists - FileUtils.mv( - File.join(Settings.absolute(storages['default'].legacy_disk_path), project_b.repository.disk_path + '.git'), - Rails.root.join(storages['test_second_storage'].legacy_disk_path, project_b.repository.disk_path + '.git') - ) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + FileUtils.mv( + File.join(Settings.absolute(storages['default'].legacy_disk_path), project_b.repository.disk_path + '.git'), + Rails.root.join(storages['test_second_storage'].legacy_disk_path, project_b.repository.disk_path + '.git') + ) + end expect { run_rake_task('gitlab:backup:create') }.to output.to_stdout diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index 01166865e88..4503288e410 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -360,13 +360,6 @@ describe ObjectStorage do subject { uploader_class.workhorse_authorize(has_length: has_length, maximum_size: maximum_size) } - before do - # ensure that we use regular Fog libraries - # other tests might call `Fog.mock!` and - # it will make tests to fail - Fog.unmock! - end - shared_examples 'uses local storage' do it "returns temporary path" do is_expected.to have_key(:TempPath) @@ -739,4 +732,26 @@ describe ObjectStorage do end end end + + describe '#retrieve_from_store!' do + [:group, :project, :user].each do |model| + context "for #{model}s" do + let(:models) { create_list(model, 3, :with_avatar).map(&:reload) } + let(:avatars) { models.map(&:avatar) } + + it 'batches fetching uploads from the database' do + # Ensure that these are all created and fully loaded before we start + # running queries for avatars + models + + expect { avatars }.not_to exceed_query_limit(1) + end + + it 'fetches a unique upload for each model' do + expect(avatars.map(&:url).uniq).to eq(avatars.map(&:url)) + expect(avatars.map(&:upload).uniq).to eq(avatars.map(&:upload)) + end + end + end + end end diff --git a/spec/uploaders/workers/object_storage/background_move_worker_spec.rb b/spec/uploaders/workers/object_storage/background_move_worker_spec.rb index b34f427fd8a..95813d15e52 100644 --- a/spec/uploaders/workers/object_storage/background_move_worker_spec.rb +++ b/spec/uploaders/workers/object_storage/background_move_worker_spec.rb @@ -125,8 +125,10 @@ describe ObjectStorage::BackgroundMoveWorker do it "migrates file to remote storage" do perform + project.reload + BatchLoader::Executor.clear_current - expect(project.reload.avatar.file_storage?).to be_falsey + expect(project.avatar).not_to be_file_storage end end @@ -137,7 +139,7 @@ describe ObjectStorage::BackgroundMoveWorker do it "migrates file to remote storage" do perform - expect(project.reload.avatar.file_storage?).to be_falsey + expect(project.reload.avatar).not_to be_file_storage end end end diff --git a/spec/workers/repository_check/single_repository_worker_spec.rb b/spec/workers/repository_check/single_repository_worker_spec.rb index a021235aed6..22fc64c1536 100644 --- a/spec/workers/repository_check/single_repository_worker_spec.rb +++ b/spec/workers/repository_check/single_repository_worker_spec.rb @@ -88,7 +88,9 @@ describe RepositoryCheck::SingleRepositoryWorker do end def break_wiki(project) - break_repo(wiki_path(project)) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + break_repo(wiki_path(project)) + end end def wiki_path(project) @@ -96,7 +98,9 @@ describe RepositoryCheck::SingleRepositoryWorker do end def break_project(project) - break_repo(project.repository.path_to_repo) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + break_repo(project.repository.path_to_repo) + end end def break_repo(repo) |
