diff options
80 files changed, 1638 insertions, 335 deletions
diff --git a/Gemfile.lock b/Gemfile.lock index 16d7f63cb66..fcc0fb64897 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -846,7 +846,7 @@ GEM rubyntlm (0.6.2) rubypants (0.2.0) rubyzip (1.2.2) - rugged (0.28.2) + rugged (0.28.3.1) safe_yaml (1.0.4) sanitize (4.6.6) crass (~> 1.0.2) diff --git a/app/assets/javascripts/behaviors/shortcuts/shortcuts.js b/app/assets/javascripts/behaviors/shortcuts/shortcuts.js index eade1283513..7e3515b1f4b 100644 --- a/app/assets/javascripts/behaviors/shortcuts/shortcuts.js +++ b/app/assets/javascripts/behaviors/shortcuts/shortcuts.js @@ -4,7 +4,7 @@ import Mousetrap from 'mousetrap'; import axios from '../../lib/utils/axios_utils'; import { refreshCurrentPage, visitUrl } from '../../lib/utils/url_utility'; import findAndFollowLink from '../../lib/utils/navigation_utility'; -import { parseBoolean } from '~/lib/utils/common_utils'; +import { parseBoolean, getCspNonceValue } from '~/lib/utils/common_utils'; const defaultStopCallback = Mousetrap.stopCallback; Mousetrap.stopCallback = (e, element, combo) => { @@ -94,7 +94,7 @@ export default class Shortcuts { responseType: 'text', }) .then(({ data }) => { - $.globalEval(data); + $.globalEval(data, { nonce: getCspNonceValue() }); if (location && location.length > 0) { const results = []; diff --git a/app/assets/javascripts/registry/components/collapsible_container.vue b/app/assets/javascripts/registry/components/collapsible_container.vue index e157036871b..bfb2305c48c 100644 --- a/app/assets/javascripts/registry/components/collapsible_container.vue +++ b/app/assets/javascripts/registry/components/collapsible_container.vue @@ -84,7 +84,7 @@ export default { v-gl-modal="modalId" :title="s__('ContainerRegistry|Remove repository')" :aria-label="s__('ContainerRegistry|Remove repository')" - class="js-remove-repo" + class="js-remove-repo btn-inverted" variant="danger" > <icon name="remove" /> diff --git a/app/assets/javascripts/registry/components/table_registry.vue b/app/assets/javascripts/registry/components/table_registry.vue index a498a553908..e9067bc2b56 100644 --- a/app/assets/javascripts/registry/components/table_registry.vue +++ b/app/assets/javascripts/registry/components/table_registry.vue @@ -1,7 +1,13 @@ <script> import { mapActions } from 'vuex'; -import { GlButton, GlTooltipDirective, GlModal, GlModalDirective } from '@gitlab/ui'; -import { n__ } from '../../locale'; +import { + GlButton, + GlFormCheckbox, + GlTooltipDirective, + GlModal, + GlModalDirective, +} from '@gitlab/ui'; +import { n__, s__, sprintf } from '../../locale'; import createFlash from '../../flash'; import ClipboardButton from '../../vue_shared/components/clipboard_button.vue'; import TablePagination from '../../vue_shared/components/pagination/table_pagination.vue'; @@ -14,6 +20,7 @@ export default { components: { ClipboardButton, TablePagination, + GlFormCheckbox, GlButton, Icon, GlModal, @@ -31,33 +38,98 @@ export default { }, data() { return { - itemToBeDeleted: null, + itemsToBeDeleted: [], modalId: `confirm-image-deletion-modal-${this.repo.id}`, + selectAllChecked: false, + modalDescription: '', }; }, computed: { + bulkDeletePath() { + return this.repo.tagsPath ? this.repo.tagsPath.replace('?format=json', '/bulk_destroy') : ''; + }, shouldRenderPagination() { return this.repo.pagination.total > this.repo.pagination.perPage; }, + modalTitle() { + return n__( + 'ContainerRegistry|Remove image', + 'ContainerRegistry|Remove images', + this.itemsToBeDeleted.length === 0 ? 1 : this.itemsToBeDeleted.length, + ); + }, + }, + mounted() { + this.$refs.deleteModal.$refs.modal.$on('hide', this.removeModalEvents); }, methods: { - ...mapActions(['fetchList', 'deleteItem']), + ...mapActions(['fetchList', 'deleteItem', 'multiDeleteItems']), + setModalDescription(itemIndex = -1) { + if (itemIndex === -1) { + this.modalDescription = sprintf( + s__(`ContainerRegistry|You are about to delete <b>%{count}</b> images. This will + delete the images and all tags pointing to them.`), + { count: this.itemsToBeDeleted.length }, + ); + } else { + const { tag } = this.repo.list[itemIndex]; + + this.modalDescription = sprintf( + s__(`ContainerRegistry|You are about to delete the image <b>%{title}</b>. This will + delete the image and all tags pointing to this image.`), + { title: `${this.repo.name}:${tag}` }, + ); + } + }, layers(item) { return item.layers ? n__('%d layer', '%d layers', item.layers) : ''; }, formatSize(size) { return numberToHumanSize(size); }, - setItemToBeDeleted(item) { - this.itemToBeDeleted = item; + removeModalEvents() { + this.$refs.deleteModal.$refs.modal.$off('ok'); + }, + deleteSingleItem(index) { + this.setModalDescription(index); + + this.$refs.deleteModal.$refs.modal.$once('ok', () => { + this.removeModalEvents(); + this.handleSingleDelete(this.repo.list[index]); + }); + }, + deleteMultipleItems() { + if (this.itemsToBeDeleted.length === 1) { + this.setModalDescription(this.itemsToBeDeleted[0]); + } else if (this.itemsToBeDeleted.length > 1) { + this.setModalDescription(); + } + + this.$refs.deleteModal.$refs.modal.$once('ok', () => { + this.removeModalEvents(); + this.handleMultipleDelete(); + }); }, - handleDeleteRegistry() { - const { itemToBeDeleted } = this; - this.itemToBeDeleted = null; - this.deleteItem(itemToBeDeleted) + handleSingleDelete(itemToDelete) { + this.deleteItem(itemToDelete) .then(() => this.fetchList({ repo: this.repo })) .catch(() => this.showError(errorMessagesTypes.DELETE_REGISTRY)); }, + handleMultipleDelete() { + const { itemsToBeDeleted } = this; + this.itemsToBeDeleted = []; + + if (this.bulkDeletePath) { + this.multiDeleteItems({ + path: this.bulkDeletePath, + items: itemsToBeDeleted.map(x => this.repo.list[x].tag), + }) + .then(() => this.fetchList({ repo: this.repo })) + .catch(() => this.showError(errorMessagesTypes.DELETE_REGISTRY)); + } else { + this.showError(errorMessagesTypes.DELETE_REGISTRY); + } + }, onPageChange(pageNumber) { this.fetchList({ repo: this.repo, page: pageNumber }).catch(() => this.showError(errorMessagesTypes.FETCH_REGISTRY), @@ -66,6 +138,35 @@ export default { showError(message) { createFlash(errorMessages[message]); }, + onSelectAllChange() { + if (this.selectAllChecked) { + this.deselectAll(); + } else { + this.selectAll(); + } + }, + selectAll() { + this.itemsToBeDeleted = this.repo.list.map((x, index) => index); + this.selectAllChecked = true; + }, + deselectAll() { + this.itemsToBeDeleted = []; + this.selectAllChecked = false; + }, + updateItemsToBeDeleted(index) { + const delIndex = this.itemsToBeDeleted.findIndex(x => x === index); + + if (delIndex > -1) { + this.itemsToBeDeleted.splice(delIndex, 1); + this.selectAllChecked = false; + } else { + this.itemsToBeDeleted.push(index); + + if (this.itemsToBeDeleted.length === this.repo.list.length) { + this.selectAllChecked = true; + } + } + }, }, }; </script> @@ -74,15 +175,44 @@ export default { <table class="table tags"> <thead> <tr> + <th> + <gl-form-checkbox + v-if="repo.canDelete" + class="js-select-all-checkbox" + :checked="selectAllChecked" + @change="onSelectAllChange" + /> + </th> <th>{{ s__('ContainerRegistry|Tag') }}</th> <th>{{ s__('ContainerRegistry|Tag ID') }}</th> <th>{{ s__('ContainerRegistry|Size') }}</th> <th>{{ s__('ContainerRegistry|Last Updated') }}</th> - <th></th> + <th> + <gl-button + v-if="repo.canDelete" + v-gl-tooltip + v-gl-modal="modalId" + :disabled="!itemsToBeDeleted || itemsToBeDeleted.length === 0" + class="js-delete-registry float-right" + variant="danger" + :title="s__('ContainerRegistry|Remove selected images')" + :aria-label="s__('ContainerRegistry|Remove selected images')" + @click="deleteMultipleItems()" + ><icon name="remove" + /></gl-button> + </th> </tr> </thead> <tbody> - <tr v-for="item in repo.list" :key="item.tag"> + <tr v-for="(item, index) in repo.list" :key="item.tag" class="registry-image-row"> + <td class="check"> + <gl-form-checkbox + v-if="item.canDelete" + class="js-select-checkbox" + :checked="itemsToBeDeleted && itemsToBeDeleted.includes(index)" + @change="updateItemsToBeDeleted(index)" + /> + </td> <td class="monospace"> {{ item.tag }} <clipboard-button @@ -111,16 +241,15 @@ export default { </span> </td> - <td class="content"> + <td class="content action-buttons"> <gl-button v-if="item.canDelete" - v-gl-tooltip v-gl-modal="modalId" :title="s__('ContainerRegistry|Remove image')" :aria-label="s__('ContainerRegistry|Remove image')" variant="danger" - class="js-delete-registry d-none d-sm-block float-right" - @click="setItemToBeDeleted(item)" + class="js-delete-registry-row float-right btn-inverted btn-border-color btn-icon" + @click="deleteSingleItem(index)" > <icon name="remove" /> </gl-button> @@ -135,19 +264,10 @@ export default { :page-info="repo.pagination" /> - <gl-modal :modal-id="modalId" ok-variant="danger" @ok="handleDeleteRegistry"> - <template v-slot:modal-title>{{ s__('ContainerRegistry|Remove image') }}</template> - <template v-slot:modal-ok>{{ s__('ContainerRegistry|Remove image and tags') }}</template> - <p - v-html=" - sprintf( - s__( - 'ContainerRegistry|You are about to delete the image <b>%{title}</b>. This will delete the image and all tags pointing to this image.', - ), - { title: repo.name }, - ) - " - ></p> + <gl-modal ref="deleteModal" :modal-id="modalId" ok-variant="danger"> + <template v-slot:modal-title>{{ modalTitle }}</template> + <template v-slot:modal-ok>{{ s__('ContainerRegistry|Remove image(s) and tags') }}</template> + <p v-html="modalDescription"></p> </gl-modal> </div> </template> diff --git a/app/assets/javascripts/registry/stores/actions.js b/app/assets/javascripts/registry/stores/actions.js index 0f5e9cc73a0..a2e0130e79e 100644 --- a/app/assets/javascripts/registry/stores/actions.js +++ b/app/assets/javascripts/registry/stores/actions.js @@ -36,6 +36,8 @@ export const fetchList = ({ commit }, { repo, page }) => { }; export const deleteItem = (_, item) => axios.delete(item.destroyPath); +export const multiDeleteItems = (_, { path, items }) => + axios.delete(path, { params: { ids: items } }); export const setMainEndpoint = ({ commit }, data) => commit(types.SET_MAIN_ENDPOINT, data); export const toggleLoading = ({ commit }) => commit(types.TOGGLE_MAIN_LOADING); diff --git a/app/assets/stylesheets/pages/container_registry.scss b/app/assets/stylesheets/pages/container_registry.scss index a21fa29f34a..0f4bdb219a3 100644 --- a/app/assets/stylesheets/pages/container_registry.scss +++ b/app/assets/stylesheets/pages/container_registry.scss @@ -31,4 +31,21 @@ .table.tags { margin-bottom: 0; + + .registry-image-row { + .check { + padding-right: $gl-padding; + width: 5%; + } + + .action-buttons { + opacity: 0; + } + + &:hover { + .action-buttons { + opacity: 1; + } + } + } } diff --git a/app/assets/stylesheets/pages/members.scss b/app/assets/stylesheets/pages/members.scss index 45408c9ab3c..ae92a2fbd7b 100644 --- a/app/assets/stylesheets/pages/members.scss +++ b/app/assets/stylesheets/pages/members.scss @@ -58,11 +58,6 @@ } } -.member-access-text { - margin-left: auto; - line-height: 43px; -} - .member-search-btn { position: absolute; right: 4px; diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 5472ef05d7c..886d1f99d69 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -176,6 +176,7 @@ class GroupsController < Groups::ApplicationController [ :avatar, :description, + :emails_disabled, :lfs_enabled, :name, :path, diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb index 956093b972b..abf8407a51c 100644 --- a/app/controllers/projects/git_http_client_controller.rb +++ b/app/controllers/projects/git_http_client_controller.rb @@ -49,7 +49,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController send_final_spnego_response return # Allow access end - elsif project && download_request? && Guest.can?(:download_code, project) + elsif project && download_request? && http_allowed? && Guest.can?(:download_code, project) + @authentication_result = Gitlab::Auth::Result.new(nil, project, :none, [:download_code]) return # Allow access @@ -113,4 +114,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController def ci? authentication_result.ci?(project) end + + def http_allowed? + Gitlab::ProtocolAccess.allowed?('http') + end end diff --git a/app/controllers/projects/registry/tags_controller.rb b/app/controllers/projects/registry/tags_controller.rb index bf1d8d8b5fc..54e2faa2dd7 100644 --- a/app/controllers/projects/registry/tags_controller.rb +++ b/app/controllers/projects/registry/tags_controller.rb @@ -5,6 +5,8 @@ module Projects class TagsController < ::Projects::Registry::ApplicationController before_action :authorize_destroy_container_image!, only: [:destroy] + LIMIT = 15 + def index respond_to do |format| format.json do @@ -28,10 +30,40 @@ module Projects end end + def bulk_destroy + unless params[:ids].present? + head :bad_request + return + end + + tag_names = params[:ids] || [] + if tag_names.size > LIMIT + head :bad_request + return + end + + @tags = tag_names.map { |tag_name| image.tag(tag_name) } + unless @tags.all? { |tag| tag.valid_name? } + head :bad_request + return + end + + success_count = 0 + @tags.each do |tag| + if tag.delete + success_count += 1 + end + end + + respond_to do |format| + format.json { head(success_count == @tags.size ? :no_content : :bad_request) } + end + end + private def tags - Kaminari::PaginatableArray.new(image.tags, limit: 15) + Kaminari::PaginatableArray.new(image.tags, limit: LIMIT) end def image diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index d4ff72c2314..e04cbf10470 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -361,6 +361,7 @@ class ProjectsController < Projects::ApplicationController :container_registry_enabled, :default_branch, :description, + :emails_disabled, :external_authorization_classification_label, :import_url, :issues_tracker, diff --git a/app/models/analytics/cycle_analytics.rb b/app/models/analytics/cycle_analytics.rb new file mode 100644 index 00000000000..626fc91cc41 --- /dev/null +++ b/app/models/analytics/cycle_analytics.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Analytics + module CycleAnalytics + def self.table_name_prefix + 'analytics_cycle_analytics_' + end + end +end diff --git a/app/models/analytics/cycle_analytics/project_stage.rb b/app/models/analytics/cycle_analytics/project_stage.rb new file mode 100644 index 00000000000..88c8cb40ccb --- /dev/null +++ b/app/models/analytics/cycle_analytics/project_stage.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Analytics + module CycleAnalytics + class ProjectStage < ApplicationRecord + belongs_to :project + end + end +end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 4306dd9266f..bfd636fa62a 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -220,18 +220,7 @@ class MergeRequest < ApplicationRecord end def rebase_in_progress? - (rebase_jid.present? && Gitlab::SidekiqStatus.running?(rebase_jid)) || - gitaly_rebase_in_progress? - end - - # TODO: remove the Gitaly lookup after v12.1, when rebase_jid will be reliable - def gitaly_rebase_in_progress? - strong_memoize(:gitaly_rebase_in_progress) do - # The source project can be deleted - next false unless source_project - - source_project.repository.rebase_in_progress?(id) - end + rebase_jid.present? && Gitlab::SidekiqStatus.running?(rebase_jid) end # Use this method whenever you need to make sure the head_pipeline is synced with the diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 058350b16ce..9f9c4288667 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -172,6 +172,13 @@ class Namespace < ApplicationRecord end end + # any ancestor can disable emails for all descendants + def emails_disabled? + strong_memoize(:emails_disabled) do + Feature.enabled?(:emails_disabled, self, default_enabled: true) && self_and_ancestors.where(emails_disabled: true).exists? + end + end + def lfs_enabled? # User namespace will always default to the global setting Gitlab.config.lfs.enabled diff --git a/app/models/notification_recipient.rb b/app/models/notification_recipient.rb index a7f73c0f29c..8e44e3d8e17 100644 --- a/app/models/notification_recipient.rb +++ b/app/models/notification_recipient.rb @@ -4,6 +4,7 @@ class NotificationRecipient include Gitlab::Utils::StrongMemoize attr_reader :user, :type, :reason + def initialize(user, type, **opts) unless NotificationSetting.levels.key?(type) || type == :subscription raise ArgumentError, "invalid type: #{type.inspect}" @@ -30,6 +31,7 @@ class NotificationRecipient def notifiable? return false unless has_access? + return false if emails_disabled? return false if own_activity? # even users with :disabled notifications receive manual subscriptions @@ -109,6 +111,12 @@ class NotificationRecipient private + # They are disabled if the project or group has disallowed it. + # No need to check the group if there is already a project + def emails_disabled? + @project ? @project.emails_disabled? : @group&.emails_disabled? + end + def read_ability return if @skip_read_ability return @read_ability if instance_variable_defined?(:@read_ability) diff --git a/app/models/project.rb b/app/models/project.rb index 0c57ed3e43a..8efe4b06f87 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -283,6 +283,7 @@ class Project < ApplicationRecord has_one :ci_cd_settings, class_name: 'ProjectCiCdSetting', inverse_of: :project, autosave: true, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :remote_mirrors, inverse_of: :project + has_many :cycle_analytics_stages, class_name: 'Analytics::CycleAnalytics::ProjectStage' accepts_nested_attributes_for :variables, allow_destroy: true accepts_nested_attributes_for :project_feature, update_only: true @@ -631,6 +632,13 @@ class Project < ApplicationRecord alias_method :ancestors, :ancestors_upto + def emails_disabled? + strong_memoize(:emails_disabled) do + # disabling in the namespace overrides the project setting + Feature.enabled?(:emails_disabled, self, default_enabled: true) && (super || namespace.emails_disabled?) + end + end + def lfs_enabled? return namespace.lfs_enabled? if self[:lfs_enabled].nil? diff --git a/app/models/project_services/emails_on_push_service.rb b/app/models/project_services/emails_on_push_service.rb index 45de64a9990..8ca40138a8f 100644 --- a/app/models/project_services/emails_on_push_service.rb +++ b/app/models/project_services/emails_on_push_service.rb @@ -24,6 +24,7 @@ class EmailsOnPushService < Service def execute(push_data) return unless supported_events.include?(push_data[:object_kind]) + return if project.emails_disabled? EmailsOnPushWorker.perform_async( project_id, diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 52c944491bf..c686e7763bb 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -92,6 +92,7 @@ class GroupPolicy < BasePolicy enable :change_visibility_level enable :set_note_created_at + enable :set_emails_disabled end rule { can?(:read_nested_project_resources) }.policy do diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index e79bac6bee3..b8dee1b0789 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -162,6 +162,7 @@ class ProjectPolicy < BasePolicy enable :set_issue_created_at enable :set_issue_updated_at enable :set_note_created_at + enable :set_emails_disabled end rule { can?(:guest_access) }.policy do diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index 73e1e00dc33..116756bacfe 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -46,6 +46,11 @@ module Groups params.delete(:parent_id) end + # overridden in EE + def remove_unallowed_params + params.delete(:emails_disabled) unless can?(current_user, :set_emails_disabled, group) + end + def valid_share_with_group_lock_change? return true unless changing_share_with_group_lock? return true if can?(current_user, :change_share_with_group_lock, group) diff --git a/app/services/merge_requests/rebase_service.rb b/app/services/merge_requests/rebase_service.rb index 8d3b9b05819..27c16ba1777 100644 --- a/app/services/merge_requests/rebase_service.rb +++ b/app/services/merge_requests/rebase_service.rb @@ -15,7 +15,8 @@ module MergeRequests end def rebase - if merge_request.gitaly_rebase_in_progress? + # Ensure Gitaly isn't already running a rebase + if source_project.repository.rebase_in_progress?(merge_request.id) log_error('Rebase task canceled: Another rebase is already in progress', save_message_on_model: true) return false end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 21fab22e0d4..83710ffce2f 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -321,6 +321,9 @@ class NotificationService end def decline_project_invite(project_member) + # Must always send, regardless of project/namespace configuration since it's a + # response to the user's action. + mailer.member_invite_declined_email( project_member.real_source_type, project_member.project.id, @@ -351,8 +354,8 @@ class NotificationService end def decline_group_invite(group_member) - # always send this one, since it's a response to the user's own - # action + # Must always send, regardless of project/namespace configuration since it's a + # response to the user's action. mailer.member_invite_declined_email( group_member.real_source_type, @@ -410,6 +413,10 @@ class NotificationService end def pipeline_finished(pipeline, recipients = nil) + # Must always check project configuration since recipients could be a list of emails + # from the PipelinesEmailService integration. + return if pipeline.project.emails_disabled? + email_template = "pipeline_#{pipeline.status}_email" return unless mailer.respond_to?(email_template) @@ -428,6 +435,8 @@ class NotificationService end def autodevops_disabled(pipeline, recipients) + return if pipeline.project.emails_disabled? + recipients.each do |recipient| mailer.autodevops_disabled_email(pipeline, recipient).deliver_later end @@ -472,10 +481,14 @@ class NotificationService end def repository_cleanup_success(project, user) + return if project.emails_disabled? + mailer.send(:repository_cleanup_success_email, project, user).deliver_later end def repository_cleanup_failure(project, user, error) + return if project.emails_disabled? + mailer.send(:repository_cleanup_failure_email, project, user, error).deliver_later end diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index caab946174d..8acbdc7e02b 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -9,6 +9,7 @@ module Projects # rubocop: disable CodeReuse/ActiveRecord def execute + remove_unallowed_params validate! ensure_wiki_exists if enabling_wiki? @@ -54,6 +55,10 @@ module Projects end end + def remove_unallowed_params + params.delete(:emails_disabled) unless can?(current_user, :set_emails_disabled, project) + end + def after_update todos_features_changes = %w( issues_access_level diff --git a/changelogs/unreleased/24705-multi-selection-for-delete-on-registry-page.yml b/changelogs/unreleased/24705-multi-selection-for-delete-on-registry-page.yml new file mode 100644 index 00000000000..5254bd36b9c --- /dev/null +++ b/changelogs/unreleased/24705-multi-selection-for-delete-on-registry-page.yml @@ -0,0 +1,5 @@ +--- +title: Added multi-select deletion of container registry images +merge_request: 30837 +author: +type: other diff --git a/changelogs/unreleased/50020-allow-email-notifications-to-be-disabled-for-all-users-of-a-group.yml b/changelogs/unreleased/50020-allow-email-notifications-to-be-disabled-for-all-users-of-a-group.yml new file mode 100644 index 00000000000..9137e9339aa --- /dev/null +++ b/changelogs/unreleased/50020-allow-email-notifications-to-be-disabled-for-all-users-of-a-group.yml @@ -0,0 +1,5 @@ +--- +title: Allow email notifications to be disabled for all members of a group or project +merge_request: 30755 +author: Dustin Spicuzza +type: added diff --git a/changelogs/unreleased/dblessing-fix-public-project-ssh-only-ci-failure.yml b/changelogs/unreleased/dblessing-fix-public-project-ssh-only-ci-failure.yml new file mode 100644 index 00000000000..615a1571e95 --- /dev/null +++ b/changelogs/unreleased/dblessing-fix-public-project-ssh-only-ci-failure.yml @@ -0,0 +1,5 @@ +--- +title: Allow CI to clone public projects when HTTP protocol is disabled +merge_request: 31632 +author: +type: fixed diff --git a/changelogs/unreleased/enable-specific-embeds.yml b/changelogs/unreleased/enable-specific-embeds.yml new file mode 100644 index 00000000000..f2e591621a8 --- /dev/null +++ b/changelogs/unreleased/enable-specific-embeds.yml @@ -0,0 +1,5 @@ +--- +title: Enable embedding of specific metrics charts in GFM +merge_request: 31304 +author: +type: added diff --git a/changelogs/unreleased/new-cycle-analytics-backend-migrations.yml b/changelogs/unreleased/new-cycle-analytics-backend-migrations.yml new file mode 100644 index 00000000000..d56a07fe569 --- /dev/null +++ b/changelogs/unreleased/new-cycle-analytics-backend-migrations.yml @@ -0,0 +1,5 @@ +--- +title: Create database tables for the new cycle analytics backend +merge_request: 31621 +author: +type: other diff --git a/changelogs/unreleased/sh-fix-discussions-api-perf.yml b/changelogs/unreleased/sh-fix-discussions-api-perf.yml new file mode 100644 index 00000000000..8cdbbf03dab --- /dev/null +++ b/changelogs/unreleased/sh-fix-discussions-api-perf.yml @@ -0,0 +1,5 @@ +--- +title: Eliminate many Gitaly calls in discussions API +merge_request: 31834 +author: +type: performance diff --git a/changelogs/unreleased/sh-update-rugged-0-28-3.yml b/changelogs/unreleased/sh-update-rugged-0-28-3.yml new file mode 100644 index 00000000000..86446564e12 --- /dev/null +++ b/changelogs/unreleased/sh-update-rugged-0-28-3.yml @@ -0,0 +1,5 @@ +--- +title: Upgrade Rugged to 0.28.3 +merge_request: 31794 +author: +type: security diff --git a/config/routes/project.rb b/config/routes/project.rb index a207ee44d47..9a453d101a1 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -477,7 +477,11 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do # in JSON format, or a request for tag named `latest.json`. scope format: false do resources :tags, only: [:index, :destroy], - constraints: { id: Gitlab::Regex.container_registry_tag_regex } + constraints: { id: Gitlab::Regex.container_registry_tag_regex } do + collection do + delete :bulk_destroy + end + end end end end diff --git a/config/routes/user.rb b/config/routes/user.rb index 3f768d5d384..d4616c8080d 100644 --- a/config/routes/user.rb +++ b/config/routes/user.rb @@ -43,13 +43,6 @@ scope '-/users', module: :users do end end -scope '-/users', module: :users do - resources :terms, only: [:index] do - post :accept, on: :member - post :decline, on: :member - end -end - scope(constraints: { username: Gitlab::PathRegex.root_namespace_route_regex }) do scope(path: 'users/:username', as: :user, diff --git a/db/migrate/20190715215532_add_project_emails_disabled.rb b/db/migrate/20190715215532_add_project_emails_disabled.rb new file mode 100644 index 00000000000..536ea34c0fb --- /dev/null +++ b/db/migrate/20190715215532_add_project_emails_disabled.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddProjectEmailsDisabled < ActiveRecord::Migration[5.2] + DOWNTIME = false + + def change + add_column :projects, :emails_disabled, :boolean + end +end diff --git a/db/migrate/20190715215549_add_group_emails_disabled.rb b/db/migrate/20190715215549_add_group_emails_disabled.rb new file mode 100644 index 00000000000..d3fd4d2d923 --- /dev/null +++ b/db/migrate/20190715215549_add_group_emails_disabled.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddGroupEmailsDisabled < ActiveRecord::Migration[5.2] + DOWNTIME = false + + def change + add_column :namespaces, :emails_disabled, :boolean + end +end diff --git a/db/migrate/20190716144222_create_analytics_cycle_analytics_project_stages.rb b/db/migrate/20190716144222_create_analytics_cycle_analytics_project_stages.rb new file mode 100644 index 00000000000..5c005377b00 --- /dev/null +++ b/db/migrate/20190716144222_create_analytics_cycle_analytics_project_stages.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class CreateAnalyticsCycleAnalyticsProjectStages < ActiveRecord::Migration[5.2] + DOWNTIME = false + + INDEX_PREFIX = 'index_analytics_ca_project_stages_' + + def change + create_table :analytics_cycle_analytics_project_stages do |t| + t.timestamps_with_timezone + t.integer :relative_position + t.integer :start_event_identifier, null: false + t.integer :end_event_identifier, null: false + t.references(:project, { + null: false, + foreign_key: { to_table: :projects, on_delete: :cascade }, + index: { name: INDEX_PREFIX + 'on_project_id' } + }) + t.references(:start_event_label, { + foreign_key: { to_table: :labels, on_delete: :cascade }, + index: { name: INDEX_PREFIX + 'on_start_event_label_id' } + }) + t.references(:end_event_label, { + foreign_key: { to_table: :labels, on_delete: :cascade }, + index: { name: INDEX_PREFIX + 'on_end_event_label_id' } + }) + t.boolean :hidden, default: false, null: false + t.boolean :custom, default: true, null: false + t.string :name, null: false, limit: 255 + end + + add_index :analytics_cycle_analytics_project_stages, [:project_id, :name], unique: true, name: INDEX_PREFIX + 'on_project_id_and_name' + add_index :analytics_cycle_analytics_project_stages, [:relative_position], name: INDEX_PREFIX + 'on_relative_position' + end +end diff --git a/db/migrate/20190729062536_create_analytics_cycle_analytics_group_stages.rb b/db/migrate/20190729062536_create_analytics_cycle_analytics_group_stages.rb new file mode 100644 index 00000000000..5b327dc5332 --- /dev/null +++ b/db/migrate/20190729062536_create_analytics_cycle_analytics_group_stages.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class CreateAnalyticsCycleAnalyticsGroupStages < ActiveRecord::Migration[5.2] + DOWNTIME = false + + INDEX_PREFIX = 'index_analytics_ca_group_stages_' + + def change + create_table :analytics_cycle_analytics_group_stages do |t| + t.timestamps_with_timezone + t.integer :relative_position + t.integer :start_event_identifier, null: false + t.integer :end_event_identifier, null: false + t.references(:group, { + null: false, + foreign_key: { to_table: :namespaces, on_delete: :cascade }, + index: { name: INDEX_PREFIX + 'on_group_id' } + }) + t.references(:start_event_label, { + foreign_key: { to_table: :labels, on_delete: :cascade }, + index: { name: INDEX_PREFIX + 'on_start_event_label_id' } + }) + t.references(:end_event_label, { + foreign_key: { to_table: :labels, on_delete: :cascade }, + index: { name: INDEX_PREFIX + 'on_end_event_label_id' } + }) + t.boolean :hidden, default: false, null: false + t.boolean :custom, default: true, null: false + t.string :name, null: false, limit: 255 + end + + add_index :analytics_cycle_analytics_group_stages, [:group_id, :name], unique: true, name: INDEX_PREFIX + 'on_group_id_and_name' + add_index :analytics_cycle_analytics_group_stages, [:relative_position], name: INDEX_PREFIX + 'on_relative_position' + end +end diff --git a/db/schema.rb b/db/schema.rb index 7c4a91da750..cf7f0fc0a3d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -26,6 +26,44 @@ ActiveRecord::Schema.define(version: 2019_08_12_070645) do t.integer "cached_markdown_version" end + create_table "analytics_cycle_analytics_group_stages", force: :cascade do |t| + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false + t.integer "relative_position" + t.integer "start_event_identifier", null: false + t.integer "end_event_identifier", null: false + t.bigint "group_id", null: false + t.bigint "start_event_label_id" + t.bigint "end_event_label_id" + t.boolean "hidden", default: false, null: false + t.boolean "custom", default: true, null: false + t.string "name", limit: 255, null: false + t.index ["end_event_label_id"], name: "index_analytics_ca_group_stages_on_end_event_label_id" + t.index ["group_id", "name"], name: "index_analytics_ca_group_stages_on_group_id_and_name", unique: true + t.index ["group_id"], name: "index_analytics_ca_group_stages_on_group_id" + t.index ["relative_position"], name: "index_analytics_ca_group_stages_on_relative_position" + t.index ["start_event_label_id"], name: "index_analytics_ca_group_stages_on_start_event_label_id" + end + + create_table "analytics_cycle_analytics_project_stages", force: :cascade do |t| + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false + t.integer "relative_position" + t.integer "start_event_identifier", null: false + t.integer "end_event_identifier", null: false + t.bigint "project_id", null: false + t.bigint "start_event_label_id" + t.bigint "end_event_label_id" + t.boolean "hidden", default: false, null: false + t.boolean "custom", default: true, null: false + t.string "name", limit: 255, null: false + t.index ["end_event_label_id"], name: "index_analytics_ca_project_stages_on_end_event_label_id" + t.index ["project_id", "name"], name: "index_analytics_ca_project_stages_on_project_id_and_name", unique: true + t.index ["project_id"], name: "index_analytics_ca_project_stages_on_project_id" + t.index ["relative_position"], name: "index_analytics_ca_project_stages_on_relative_position" + t.index ["start_event_label_id"], name: "index_analytics_ca_project_stages_on_start_event_label_id" + end + create_table "appearances", id: :serial, force: :cascade do |t| t.string "title", null: false t.text "description", null: false @@ -2175,6 +2213,7 @@ ActiveRecord::Schema.define(version: 2019_08_12_070645) do t.boolean "membership_lock", default: false t.integer "last_ci_minutes_usage_notification_level" t.integer "subgroup_creation_level", default: 1 + t.boolean "emails_disabled" t.index ["created_at"], name: "index_namespaces_on_created_at" t.index ["custom_project_templates_group_id", "type"], name: "index_namespaces_on_custom_project_templates_group_id_and_type", where: "(custom_project_templates_group_id IS NOT NULL)" t.index ["file_template_project_id"], name: "index_namespaces_on_file_template_project_id" @@ -2745,6 +2784,7 @@ ActiveRecord::Schema.define(version: 2019_08_12_070645) do t.boolean "reset_approvals_on_push", default: true t.boolean "service_desk_enabled", default: true t.integer "approvals_before_merge", default: 0, null: false + t.boolean "emails_disabled" t.index ["archived", "pending_delete", "merge_requests_require_code_owner_approval"], name: "projects_requiring_code_owner_approval", where: "((pending_delete = false) AND (archived = false) AND (merge_requests_require_code_owner_approval = true))" t.index ["created_at"], name: "index_projects_on_created_at" t.index ["creator_id"], name: "index_projects_on_creator_id" @@ -3630,6 +3670,12 @@ ActiveRecord::Schema.define(version: 2019_08_12_070645) do t.index ["type"], name: "index_web_hooks_on_type" end + add_foreign_key "analytics_cycle_analytics_group_stages", "labels", column: "end_event_label_id", on_delete: :cascade + add_foreign_key "analytics_cycle_analytics_group_stages", "labels", column: "start_event_label_id", on_delete: :cascade + add_foreign_key "analytics_cycle_analytics_group_stages", "namespaces", column: "group_id", on_delete: :cascade + add_foreign_key "analytics_cycle_analytics_project_stages", "labels", column: "end_event_label_id", on_delete: :cascade + add_foreign_key "analytics_cycle_analytics_project_stages", "labels", column: "start_event_label_id", on_delete: :cascade + add_foreign_key "analytics_cycle_analytics_project_stages", "projects", on_delete: :cascade add_foreign_key "application_settings", "namespaces", column: "custom_project_templates_group_id", on_delete: :nullify add_foreign_key "application_settings", "projects", column: "file_template_project_id", name: "fk_ec757bd087", on_delete: :nullify add_foreign_key "application_settings", "projects", column: "instance_administration_project_id", on_delete: :nullify diff --git a/doc/ci/directed_acyclic_graph/index.md b/doc/ci/directed_acyclic_graph/index.md new file mode 100644 index 00000000000..e54be9f3bd9 --- /dev/null +++ b/doc/ci/directed_acyclic_graph/index.md @@ -0,0 +1,76 @@ +--- +type: reference +--- + +# Directed Acyclic Graph + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/47063) in GitLab 12.2 (enabled by `ci_dag_support` feature flag). + +A [directed acyclic graph](https://www.techopedia.com/definition/5739/directed-acyclic-graph-dag) can be +used in the context of a CI/CD pipeline to build relationships between jobs such that +execution is performed in the quickest possible manner, regardless how stages may +be set up. + +For example, you may have a specific tool or separate website that is built +as part of your main project. Using a DAG, you can specify the relationship between +these jobs and GitLab will then execute the jobs as soon as possible instead of waiting +for each stage to complete. + +Unlike other DAG solutions for CI/CD, GitLab does not require you to choose one or the +other. You can implement a hybrid combination of DAG and traditional +stage-based operation within a single pipeline. Configuration is kept very simple, +requiring a single keyword to enable the feature for any job. + +Consider a monorepo as follows: + +``` +./service_a +./service_b +./service_c +./service_d +``` + +It has a pipeline that looks like the following: + +| build | test | deploy | +| ----- | ---- | ------ | +| build_a | test_a | deploy_a | +| build_b | test_b | deploy_b | +| build_c | test_c | deploy_c | +| build_d | test_d | deploy_d | + +Using a DAG, you can relate the `_a` jobs to each other separately from the `_b` jobs, +and even if service `a` takes a very long time to build, service `b` will not +wait for it and will finish as quickly as it can. In this very same pipeline, `_c` and +`_d` can be left alone and will run together in staged sequence just like any normal +GitLab pipeline. + +## Use cases + +A DAG can help solve several different kinds of relationships between jobs within +a CI/CD pipeline. Most typically this would cover when jobs need to fan in or out, +and/or merge back together (diamond dependencies). This can happen when you're +handling multi-platform builds or complex webs of dependencies as in something like +an operating system build or a complex deployment graph of independently deployable +but related microservices. + +Additionally, a DAG can help with general speediness of pipelines and helping +to deliver fast feedback. By creating dependency relationships that don't unnecessarily +block each other, your pipelines will run as quickly as possible regardless of +pipeline stages, ensuring output (including errors) is available to developers +as quickly as possible. + +## Usage + +Relationships are defined between jobs using the [`needs:` keyword](../yaml/README.md#needs). + +Note that `needs:` also works with the [parallel](../yaml/README.md#parallel) keyword, +giving your powerful options for parallelization within your pipeline. + +## Limitations + +A directed acyclic graph is a complicated feature, and as of the initial MVC there +are certain use cases that you may need to work around. For more information: + +- [`needs` requirements and limitations](../yaml/README.md#requirements-and-limitations). +- Related epic [gitlab-org#1716](https://gitlab.com/groups/gitlab-org/-/epics/1716). diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index a6051e87366..2be93433b36 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -1665,6 +1665,84 @@ You can ask your administrator to [flip this switch](../../administration/job_artifacts.md#validation-for-dependencies) and bring back the old behavior. +### `needs` + +> Introduced in GitLab 12.2. + +The `needs:` keyword enables executing jobs out-of-order, allowing you to implement +a [directed acyclic graph](../directed_acyclic_graph/index.md) in your `.gitlab-ci.yml`. + +This lets you run some jobs without waiting for other ones, disregarding stage ordering +so you can have multiple stages running concurrently. + +Let's consider the following example: + +```yaml +linux:build: + stage: build + +mac:build: + stage: build + +linux:rspec: + stage: test + needs: [linux:build] + +linux:rubocop: + stage: test + needs: [linux:build] + +mac:rspec: + stage: test + needs: [mac:build] + +mac:rubocop: + stage: test + needs: [mac:build] + +production: + stage: deploy +``` + +This example creates three paths of execution: + +- Linux path: the `linux:rspec` and `linux:rubocop` jobs will be run as soon + as the `linux:build` job finishes without waiting for `mac:build` to finish. + +- macOS path: the `mac:rspec` and `mac:rubocop` jobs will be run as soon + as the `mac:build` job finishes, without waiting for `linux:build` to finish. + +- The `production` job will be executed as soon as all previous jobs + finish; in this case: `linux:build`, `linux:rspec`, `linux:rubocop`, + `mac:build`, `mac:rspec`, `mac:rubocop`. + +#### Requirements and limitations + +1. If `needs:` is set to point to a job that is not instantiated + because of `only/except` rules or otherwise does not exist, the + job will fail. +1. Note that one day one of the launch, we are temporarily limiting the + maximum number of jobs that a single job can need in the `needs:` array. Track + our [infrastructure issue](https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/7541) + for details on the current limit. +1. If you use `dependencies:` with `needs:`, it's important that you + do not mark a job as having a dependency on something that won't + have been run at the time it needs it. It's better to use both + keywords in this case so that GitLab handles the ordering appropriately. +1. It is impossible for now to have `needs: []` (empty needs), + the job always needs to depend on something, unless this is the job + in the first stage (see [gitlab-ce#65504](https://gitlab.com/gitlab-org/gitlab-ce/issues/65504)). +1. If `needs:` refers to a job that is marked as `parallel:`. + the current job will depend on all parallel jobs created. +1. `needs:` is similar to `dependencies:` in that needs to use jobs from + prior stages, this means that it is impossible to create circular + dependencies or depend on jobs in the current stage (see [gitlab-ce#65505](https://gitlab.com/gitlab-org/gitlab-ce/issues/65505)). +1. Related to the above, stages must be explicitly defined for all jobs + that have the keyword `needs:` or are referred to by one. +1. For self-managed users, the feature must be turned on using the `ci_dag_support` + feature flag. The `ci_dag_limit_needs` option, if set, will limit the number of + jobs that a single job can need to `50`. If unset, the limit is `5`. + ### `coverage` > [Introduced][ce-7447] in GitLab 8.17. diff --git a/doc/user/analytics/productivity_analytics.md b/doc/user/analytics/productivity_analytics.md deleted file mode 100644 index db3e37bd0fb..00000000000 --- a/doc/user/analytics/productivity_analytics.md +++ /dev/null @@ -1,69 +0,0 @@ -# Productivity Analytics **(PREMIUM)** - -> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/12079) in [GitLab Premium](https://about.gitlab.com/pricing/) 12.2 (enabled by feature flags `analytics` and `productivity_analytics`). - -Track development velocity with Productivity Analytics. - -For many companies, the development cycle is a blackbox and getting an estimate of how -long, on average, it takes to deliver features is an enormous endeavor. - -While [Cycle Analytics](../project/cycle_analytics.md) focuses on the entire -SDLC process, Productivity Analytics provides a way for Engineering Management to -drill down in a systematic way to uncover patterns and causes for success or failure at -an individual, project or group level. - -Productivity can slow down for many reasons ranging from degrading code base to quickly -growing teams. In order to investigate, department or team leaders can start by visualizing the time -it takes for merge requests to be merged. - -## Supported features - -Productivity Analytics allows GitLab users to: - -- Visualize typical Merge Request lifetime and statistics. Use a histogram - that shows the distribution of the time elapsed between creating and merging - Merge Requests. -- Drill down into the most time consuming Merge Requests, select a number of outliers, - and filter down all subsequent charts to investigate potential causes. -- Filter by group, project, author, label, milestone, or a specific date range. - Filter down, for example, to the Merge Requests of a specific author in a group - or project during a milestone or specific date range. - -## Accessing metrics and visualizations - -To access the **Productivity Analytics** page, go to **Analytics > Productivity Analytics**. - -The following metrics and visualizations are available on a project or group level: - -- Histogram showing the number of Merge Request that took a specified number of days to - merge after creation. Select a specific column to filter down subsequent charts. -- Histogram showing a breakdown of the time taken (in hours) to merge a Merge Request. - The following intervals are available: - - Time from first commit to first comment. - - Time from first comment until last commit. - - Time from last commit to merge. -- Histogram showing the size or complexity of a Merge Request, using the following: - - Number of commits per Merge Request. - - Number of lines of code per commit. - - Number of files touched. -- Table showing list of Merge Requests with their respective times and size metrics. - Can be sorted by the above metrics. - - Users can sort by any of the above metrics - -## Retrieving data - -Users can retrieve three months of data when they deploy Productivity Analytics for the first time. - -To retrieve data for a different time span, run the following in the GitLab directory: - -```sh -MERGED_AT_AFTER = <your_date> rake gitlab:productivity_analytics:recalc -``` - -## Permissions - -The **Productivity Analytics** dashboard can be accessed only: - -- On GitLab instances and namespaces on - [Premium or Silver tier](https://about.gitlab.com/pricing/) and above. -- By users with [Reporter access](../permissions.md) and above. diff --git a/doc/user/profile/preferences.md b/doc/user/profile/preferences.md index 5bd4b263a58..82a6d2b3703 100644 --- a/doc/user/profile/preferences.md +++ b/doc/user/profile/preferences.md @@ -87,6 +87,16 @@ You have 8 options here that you can use for your default dashboard view: - Assigned Merge Requests - Operations Dashboard **(PREMIUM)** +### Group overview content + +The **Group overview content** dropdown allows you to choose what information is +displayed on a group’s home page. + +You can choose between 2 options: + +- Details (default) +- [Security dashboard](../application_security/security_dashboard/index.md) **(ULTIMATE)** + ### Project overview content The project overview content setting allows you to choose what content you want to diff --git a/doc/user/project/clusters/index.md b/doc/user/project/clusters/index.md index ffaa07cb3a4..cf3a3fef79f 100644 --- a/doc/user/project/clusters/index.md +++ b/doc/user/project/clusters/index.md @@ -173,6 +173,9 @@ Starting from [GitLab 12.1](https://gitlab.com/gitlab-org/gitlab-ce/issues/55902 ### Add existing Kubernetes cluster +NOTE: **Note:** +Kubernetes integration is not supported for arm64 clusters. See the issue [Helm Tiller fails to install on arm64 cluster](https://gitlab.com/gitlab-org/gitlab-ce/issues/64044) for details. + To add an existing Kubernetes cluster to your project: 1. Navigate to your project's **Operations > Kubernetes** page. diff --git a/lib/api/discussions.rb b/lib/api/discussions.rb index cc62ce22a1b..6c1acc3963f 100644 --- a/lib/api/discussions.rb +++ b/lib/api/discussions.rb @@ -4,6 +4,7 @@ module API class Discussions < Grape::API include PaginationParams helpers ::API::Helpers::NotesHelpers + helpers ::RendersNotes before { authenticate! } @@ -23,21 +24,15 @@ module API requires :noteable_id, types: [Integer, String], desc: 'The ID of the noteable' use :pagination end - # rubocop: disable CodeReuse/ActiveRecord + get ":id/#{noteables_path}/:noteable_id/discussions" do noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id]) - notes = noteable.notes - .inc_relations_for_view - .includes(:noteable) - .fresh - - notes = notes.reject { |n| n.cross_reference_not_visible_for?(current_user) } + notes = readable_discussion_notes(noteable) discussions = Kaminari.paginate_array(Discussion.build_collection(notes, noteable)) present paginate(discussions), with: Entities::Discussion end - # rubocop: enable CodeReuse/ActiveRecord desc "Get a single #{noteable_type.to_s.downcase} discussion" do success Entities::Discussion @@ -226,13 +221,24 @@ module API helpers do # rubocop: disable CodeReuse/ActiveRecord - def readable_discussion_notes(noteable, discussion_id) + def readable_discussion_notes(noteable, discussion_id = nil) notes = noteable.notes - .where(discussion_id: discussion_id) + notes = notes.where(discussion_id: discussion_id) if discussion_id + notes = notes .inc_relations_for_view .includes(:noteable) .fresh + # Without RendersActions#prepare_notes_for_rendering, + # Note#cross_reference_not_visible_for? will attempt to render + # Markdown references mentioned in the note to see whether they + # should be redacted. For notes that reference a commit, this + # would also incur a Gitaly call to verify the commit exists. + # + # With prepare_notes_for_rendering, we can avoid Gitaly calls + # because notes are redacted if they point to projects that + # cannot be accessed by the user. + notes = prepare_notes_for_rendering(notes) notes.reject { |n| n.cross_reference_not_visible_for?(current_user) } end # rubocop: enable CodeReuse/ActiveRecord diff --git a/lib/banzai/filter/inline_metrics_filter.rb b/lib/banzai/filter/inline_metrics_filter.rb index 0120cc37d6f..c5a328c21b2 100644 --- a/lib/banzai/filter/inline_metrics_filter.rb +++ b/lib/banzai/filter/inline_metrics_filter.rb @@ -15,17 +15,6 @@ module Banzai ) end - # Endpoint FE should hit to collect the appropriate - # chart information - def metrics_dashboard_url(params) - Gitlab::Metrics::Dashboard::Url.build_dashboard_url( - params['namespace'], - params['project'], - params['environment'], - embedded: true - ) - end - # Search params for selecting metrics links. A few # simple checks is enough to boost performance without # the cost of doing a full regex match. @@ -38,6 +27,28 @@ module Banzai def link_pattern Gitlab::Metrics::Dashboard::Url.regex end + + private + + # Endpoint FE should hit to collect the appropriate + # chart information + def metrics_dashboard_url(params) + Gitlab::Metrics::Dashboard::Url.build_dashboard_url( + params['namespace'], + params['project'], + params['environment'], + embedded: true, + **query_params(params['url']) + ) + end + + # Parses query params out from full url string into hash. + # + # Ex) 'https://<root>/<project>/<environment>/metrics?title=Title&group=Group' + # --> { title: 'Title', group: 'Group' } + def query_params(url) + Gitlab::Metrics::Dashboard::Url.parse_query(url) + end end end end diff --git a/lib/container_registry/tag.rb b/lib/container_registry/tag.rb index ef41dc560c9..ebea84fa1ca 100644 --- a/lib/container_registry/tag.rb +++ b/lib/container_registry/tag.rb @@ -6,6 +6,9 @@ module ContainerRegistry attr_reader :repository, :name + # https://github.com/docker/distribution/commit/3150937b9f2b1b5b096b2634d0e7c44d4a0f89fb + TAG_NAME_REGEX = /^[\w][\w.-]{0,127}$/.freeze + delegate :registry, :client, to: :repository delegate :revision, :short_revision, to: :config_blob, allow_nil: true @@ -13,6 +16,10 @@ module ContainerRegistry @repository, @name = repository, name end + def valid_name? + !name.match(TAG_NAME_REGEX).nil? + end + def valid? manifest.present? end diff --git a/lib/gitlab/danger/helper.rb b/lib/gitlab/danger/helper.rb index c0a12318990..332ca8bf9b8 100644 --- a/lib/gitlab/danger/helper.rb +++ b/lib/gitlab/danger/helper.rb @@ -113,7 +113,7 @@ module Gitlab yarn\.lock )\z}x => :frontend, - %r{\A(ee/)?db/} => :database, + %r{\A(ee/)?db/(?!fixtures)[^/]+} => :database, %r{\A(ee/)?lib/gitlab/(database|background_migration|sql|github_import)(/|\.rb)} => :database, %r{\A(app/models/project_authorization|app/services/users/refresh_authorized_projects_service)(/|\.rb)} => :database, %r{\Arubocop/cop/migration(/|\.rb)} => :database, diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 1b7fc5fa10f..bd0f3e70749 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -137,6 +137,7 @@ excluded_attributes: - :packages_enabled - :mirror_last_update_at - :mirror_last_successful_update_at + - :emails_disabled namespaces: - :runners_token - :runners_token_encrypted diff --git a/lib/gitlab/metrics/dashboard/url.rb b/lib/gitlab/metrics/dashboard/url.rb index b197e7ca86b..94f8b2e02b1 100644 --- a/lib/gitlab/metrics/dashboard/url.rb +++ b/lib/gitlab/metrics/dashboard/url.rb @@ -21,14 +21,26 @@ module Gitlab \/(?<environment>\d+) \/metrics (?<query> - \?[a-z0-9_=-]+ - (&[a-z0-9_=-]+)* + \?[a-zA-Z0-9%.()+_=-]+ + (&[a-zA-Z0-9%.()+_=-]+)* )? (?<anchor>\#[a-z0-9_-]+)? ) }x end + # Parses query params out from full url string into hash. + # + # Ex) 'https://<root>/<project>/<environment>/metrics?title=Title&group=Group' + # --> { title: 'Title', group: 'Group' } + def parse_query(url) + query_string = URI.parse(url).query.to_s + + CGI.parse(query_string) + .transform_values { |value| value.first } + .symbolize_keys + end + # Builds a metrics dashboard url based on the passed in arguments def build_dashboard_url(*args) Gitlab::Routing.url_helpers.metrics_dashboard_namespace_project_environment_url(*args) diff --git a/lib/gitlab/project_template.rb b/lib/gitlab/project_template.rb index dbf469a44c1..fa1d1203842 100644 --- a/lib/gitlab/project_template.rb +++ b/lib/gitlab/project_template.rb @@ -24,6 +24,14 @@ module Gitlab "#{preview}.git" end + def project_path + URI.parse(preview).path.sub(%r{\A/}, '') + end + + def uri_encoded_project_path + ERB::Util.url_encode(project_path) + end + def ==(other) name == other.name && title == other.title end diff --git a/lib/tasks/gitlab/update_templates.rake b/lib/tasks/gitlab/update_templates.rake index 8267c235a7f..fdcd34320b1 100644 --- a/lib/tasks/gitlab/update_templates.rake +++ b/lib/tasks/gitlab/update_templates.rake @@ -40,7 +40,6 @@ namespace :gitlab do templates.each do |template| params = { - import_url: template.clone_url, namespace_id: tmp_namespace.id, path: template.name, skip_wiki: true @@ -53,22 +52,46 @@ namespace :gitlab do raise "Failed to create project: #{project.errors.messages}" end - loop do - if project.import_finished? - puts "Import finished for #{template.name}" - break + uri_encoded_project_path = template.uri_encoded_project_path + + # extract a concrete commit for signing off what we actually downloaded + # this way we do the right thing even if the repository gets updated in the meantime + get_commits_response = Gitlab::HTTP.get("https://gitlab.com/api/v4/projects/#{uri_encoded_project_path}/repository/commits", + query: { page: 1, per_page: 1 } + ) + raise "Failed to retrieve latest commit for template '#{template.name}'" unless get_commits_response.success? + + commit_sha = get_commits_response.parsed_response.dig(0, 'id') + + project_archive_uri = "https://gitlab.com/api/v4/projects/#{uri_encoded_project_path}/repository/archive.tar.gz?sha=#{commit_sha}" + commit_message = <<~MSG + Initialized from '#{template.title}' project template + + Template repository: #{template.preview} + Commit SHA: #{commit_sha} + MSG + + Dir.mktmpdir do |tmpdir| + Dir.chdir(tmpdir) do + Gitlab::TaskHelpers.run_command!(['wget', project_archive_uri, '-O', 'archive.tar.gz']) + Gitlab::TaskHelpers.run_command!(['tar', 'xf', 'archive.tar.gz']) + extracted_project_basename = Dir['*/'].first + Dir.chdir(extracted_project_basename) do + Gitlab::TaskHelpers.run_command!(%w(git init)) + Gitlab::TaskHelpers.run_command!(%w(git add .)) + Gitlab::TaskHelpers.run_command!(['git', 'commit', '--author', 'GitLab <root@localhost>', '--message', commit_message]) + + # Hacky workaround to push to the project in a way that works with both GDK and the test environment + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab::TaskHelpers.run_command!(['git', 'remote', 'add', 'origin', "file://#{project.repository.raw.path}"]) + end + Gitlab::TaskHelpers.run_command!(['git', 'push', '-u', 'origin', 'master']) + end end - - if project.import_failed? - raise "Failed to import from #{project_params[:import_url]}" - end - - puts "Waiting for the import to finish" - - sleep(5) - project.reset end + project.reset + Projects::ImportExport::ExportService.new(project, admin).execute downloader.call(project.export_file, template.archive_path) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index dd69fa1f8f6..6227ca9afc3 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3178,14 +3178,19 @@ msgid "ContainerRegistry|Quick Start" msgstr "" msgid "ContainerRegistry|Remove image" -msgstr "" +msgid_plural "ContainerRegistry|Remove images" +msgstr[0] "" +msgstr[1] "" -msgid "ContainerRegistry|Remove image and tags" +msgid "ContainerRegistry|Remove image(s) and tags" msgstr "" msgid "ContainerRegistry|Remove repository" msgstr "" +msgid "ContainerRegistry|Remove selected images" +msgstr "" + msgid "ContainerRegistry|Size" msgstr "" @@ -3207,6 +3212,9 @@ msgstr "" msgid "ContainerRegistry|With the Docker Container Registry integrated into GitLab, every project can have its own space to store its Docker images. %{docLinkStart}More Information%{docLinkEnd}" msgstr "" +msgid "ContainerRegistry|You are about to delete <b>%{count}</b> images. This will delete the images and all tags pointing to them." +msgstr "" + msgid "ContainerRegistry|You are about to delete the image <b>%{title}</b>. This will delete the image and all tags pointing to this image." msgstr "" diff --git a/package.json b/package.json index 803aebcb5fd..2b9a00d1cbd 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,7 @@ "@babel/plugin-syntax-import-meta": "^7.2.0", "@babel/preset-env": "^7.4.4", "@gitlab/csslab": "^1.9.0", - "@gitlab/svgs": "^1.67.0", + "@gitlab/svgs": "^1.68.0", "@gitlab/ui": "5.15.0", "apollo-cache-inmemory": "^1.5.1", "apollo-client": "^2.5.1", diff --git a/spec/controllers/projects/git_http_controller_spec.rb b/spec/controllers/projects/git_http_controller_spec.rb index bf099e8deeb..88fa2236e33 100644 --- a/spec/controllers/projects/git_http_controller_spec.rb +++ b/spec/controllers/projects/git_http_controller_spec.rb @@ -12,4 +12,15 @@ describe Projects::GitHttpController do expect(response.status).to eq(403) end end + + describe 'GET #info_refs' do + it 'returns 401 for unauthenticated requests to public repositories when http protocol is disabled' do + stub_application_setting(enabled_git_access_protocol: 'ssh') + project = create(:project, :public, :repository) + + get :info_refs, params: { service: 'git-upload-pack', namespace_id: project.namespace.to_param, project_id: project.path + '.git' } + + expect(response.status).to eq(401) + end + end end diff --git a/spec/controllers/projects/registry/tags_controller_spec.rb b/spec/controllers/projects/registry/tags_controller_spec.rb index ff35139ae2e..c6e063d8229 100644 --- a/spec/controllers/projects/registry/tags_controller_spec.rb +++ b/spec/controllers/projects/registry/tags_controller_spec.rb @@ -113,4 +113,37 @@ describe Projects::Registry::TagsController do format: :json end end + + describe 'POST bulk_destroy' do + context 'when user has access to registry' do + before do + project.add_developer(user) + end + + context 'when there is matching tag present' do + before do + stub_container_registry_tags(repository: repository.path, tags: %w[rc1 test.]) + end + + it 'makes it possible to delete tags in bulk' do + allow_any_instance_of(ContainerRegistry::Tag).to receive(:delete) { |*args| ContainerRegistry::Tag.delete(*args) } + expect(ContainerRegistry::Tag).to receive(:delete).exactly(2).times + + bulk_destroy_tags(['rc1', 'test.']) + end + end + end + + private + + def bulk_destroy_tags(names) + post :bulk_destroy, params: { + namespace_id: project.namespace, + project_id: project, + repository_id: repository, + ids: names + }, + format: :json + end + end end diff --git a/spec/factories/services.rb b/spec/factories/services.rb index f3e662ad4f5..b2d6ada91fa 100644 --- a/spec/factories/services.rb +++ b/spec/factories/services.rb @@ -16,6 +16,19 @@ FactoryBot.define do ) end + factory :emails_on_push_service do + project + type 'EmailsOnPushService' + active true + push_events true + tag_push_events true + properties( + recipients: 'test@example.com', + disable_diffs: true, + send_from_committer_email: true + ) + end + factory :mock_deployment_service do project type 'MockDeploymentService' diff --git a/spec/features/container_registry_spec.rb b/spec/features/container_registry_spec.rb index 89dece97a35..aefdc4d6d4f 100644 --- a/spec/features/container_registry_spec.rb +++ b/spec/features/container_registry_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe "Container Registry", :js do +describe 'Container Registry', :js do let(:user) { create(:user) } let(:project) { create(:project) } @@ -40,8 +40,7 @@ describe "Container Registry", :js do it 'user removes entire container repository' do visit_container_registry - expect_any_instance_of(ContainerRepository) - .to receive(:delete_tags!).and_return(true) + expect_any_instance_of(ContainerRepository).to receive(:delete_tags!).and_return(true) click_on(class: 'js-remove-repo') expect(find('.modal .modal-title')).to have_content 'Remove repository' @@ -54,10 +53,9 @@ describe "Container Registry", :js do find('.js-toggle-repo').click wait_for_requests - expect_any_instance_of(ContainerRegistry::Tag) - .to receive(:delete).and_return(true) + expect_any_instance_of(ContainerRegistry::Tag).to receive(:delete).and_return(true) - click_on(class: 'js-delete-registry') + click_on(class: 'js-delete-registry-row', visible: false) expect(find('.modal .modal-title')).to have_content 'Remove image' find('.modal .modal-footer .btn-danger').click end diff --git a/spec/features/markdown/metrics_spec.rb b/spec/features/markdown/metrics_spec.rb index aa53ac50c78..4de67cfcdbe 100644 --- a/spec/features/markdown/metrics_spec.rb +++ b/spec/features/markdown/metrics_spec.rb @@ -26,13 +26,31 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching do restore_host end - context 'with deployments and related deployable present' do - it 'shows embedded metrics' do + it 'shows embedded metrics' do + visit project_issue_path(project, issue) + + expect(page).to have_css('div.prometheus-graph') + expect(page).to have_text('Memory Usage (Total)') + expect(page).to have_text('Core Usage (Total)') + end + + context 'when dashboard params are in included the url' do + let(:metrics_url) { metrics_project_environment_url(project, environment, **chart_params) } + + let(:chart_params) do + { + group: 'System metrics (Kubernetes)', + title: 'Memory Usage (Pod average)', + y_label: 'Memory Used per Pod (MB)' + } + end + + it 'shows embedded metrics for the specifiec chart' do visit project_issue_path(project, issue) expect(page).to have_css('div.prometheus-graph') - expect(page).to have_text('Memory Usage (Total)') - expect(page).to have_text('Core Usage (Total)') + expect(page).to have_text(chart_params[:title]) + expect(page).to have_text(chart_params[:y_label]) end end diff --git a/spec/javascripts/registry/components/table_registry_spec.js b/spec/javascripts/registry/components/table_registry_spec.js index 31ac970378e..9c7439206ef 100644 --- a/spec/javascripts/registry/components/table_registry_spec.js +++ b/spec/javascripts/registry/components/table_registry_spec.js @@ -1,61 +1,159 @@ import Vue from 'vue'; import tableRegistry from '~/registry/components/table_registry.vue'; import store from '~/registry/stores'; +import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; import { repoPropsData } from '../mock_data'; -const [firstImage] = repoPropsData.list; +const [firstImage, secondImage] = repoPropsData.list; describe('table registry', () => { let vm; - let Component; + const Component = Vue.extend(tableRegistry); + const bulkDeletePath = 'path'; const findDeleteBtn = () => vm.$el.querySelector('.js-delete-registry'); + const findDeleteBtnRow = () => vm.$el.querySelector('.js-delete-registry-row'); + const findSelectAllCheckbox = () => vm.$el.querySelector('.js-select-all-checkbox > input'); + const findAllRowCheckboxes = () => + Array.from(vm.$el.querySelectorAll('.js-select-checkbox input')); + const confirmationModal = (child = '') => document.querySelector(`#${vm.modalId} ${child}`); - beforeEach(() => { - Component = Vue.extend(tableRegistry); - vm = new Component({ + const createComponent = () => { + vm = mountComponentWithStore(Component, { store, - propsData: { + props: { repo: repoPropsData, }, - }).$mount(); + }); + }; + + const selectAllCheckboxes = () => vm.selectAll(); + const deselectAllCheckboxes = () => vm.deselectAll(); + + beforeEach(() => { + createComponent(); }); afterEach(() => { vm.$destroy(); }); - it('should render a table with the registry list', () => { - expect(vm.$el.querySelectorAll('table tbody tr').length).toEqual(repoPropsData.list.length); + describe('rendering', () => { + it('should render a table with the registry list', () => { + expect(vm.$el.querySelectorAll('table tbody tr').length).toEqual(repoPropsData.list.length); + }); + + it('should render registry tag', () => { + const textRendered = vm.$el + .querySelector('.table tbody tr') + .textContent.trim() + // replace additional whitespace characters (e.g. new lines) with a single empty space + .replace(/\s\s+/g, ' '); + + expect(textRendered).toContain(repoPropsData.list[0].tag); + expect(textRendered).toContain(repoPropsData.list[0].shortRevision); + expect(textRendered).toContain(repoPropsData.list[0].layers); + expect(textRendered).toContain(repoPropsData.list[0].size); + }); }); - it('should render registry tag', () => { - const textRendered = vm.$el - .querySelector('.table tbody tr') - .textContent.trim() - .replace(/\s\s+/g, ' '); + describe('multi select', () => { + it('should support multiselect and selecting a row should enable delete button', done => { + findSelectAllCheckbox().click(); + selectAllCheckboxes(); + + expect(findSelectAllCheckbox().checked).toBe(true); + + Vue.nextTick(() => { + expect(findDeleteBtn().disabled).toBe(false); + done(); + }); + }); + + it('selecting all checkbox should select all rows and enable delete button', done => { + selectAllCheckboxes(); + + Vue.nextTick(() => { + const checkedValues = findAllRowCheckboxes().filter(x => x.checked); + + expect(checkedValues.length).toBe(repoPropsData.list.length); + done(); + }); + }); + + it('deselecting select all checkbox should deselect all rows and disable delete button', done => { + selectAllCheckboxes(); + deselectAllCheckboxes(); + + Vue.nextTick(() => { + const checkedValues = findAllRowCheckboxes().filter(x => x.checked); + + expect(checkedValues.length).toBe(0); + done(); + }); + }); + + it('should delete multiple items when multiple items are selected', done => { + selectAllCheckboxes(); + + Vue.nextTick(() => { + expect(vm.itemsToBeDeleted).toEqual([0, 1]); + expect(findDeleteBtn().disabled).toBe(false); + + findDeleteBtn().click(); + spyOn(vm, 'multiDeleteItems').and.returnValue(Promise.resolve()); + + Vue.nextTick(() => { + const modal = confirmationModal(); + confirmationModal('.btn-danger').click(); + + expect(modal).toExist(); - expect(textRendered).toContain(repoPropsData.list[0].tag); - expect(textRendered).toContain(repoPropsData.list[0].shortRevision); - expect(textRendered).toContain(repoPropsData.list[0].layers); - expect(textRendered).toContain(repoPropsData.list[0].size); + Vue.nextTick(() => { + expect(vm.itemsToBeDeleted).toEqual([]); + expect(vm.multiDeleteItems).toHaveBeenCalledWith({ + path: bulkDeletePath, + items: [firstImage.tag, secondImage.tag], + }); + done(); + }); + }); + }); + }); }); describe('delete registry', () => { - it('should be possible to delete a registry', () => { - expect(findDeleteBtn()).toBeDefined(); + beforeEach(() => { + vm.itemsToBeDeleted = [0]; }); - it('should call deleteItem and reset itemToBeDeleted when confirming deletion', done => { - findDeleteBtn().click(); - spyOn(vm, 'deleteItem').and.returnValue(Promise.resolve()); + it('should be possible to delete a registry', done => { + Vue.nextTick(() => { + expect(vm.itemsToBeDeleted).toEqual([0]); + expect(findDeleteBtn()).toBeDefined(); + expect(findDeleteBtn().disabled).toBe(false); + expect(findDeleteBtnRow()).toBeDefined(); + done(); + }); + }); + it('should call deleteItems and reset itemsToBeDeleted when confirming deletion', done => { Vue.nextTick(() => { - document.querySelector(`#${vm.modalId} .btn-danger`).click(); + expect(vm.itemsToBeDeleted).toEqual([0]); + expect(findDeleteBtn().disabled).toBe(false); + findDeleteBtn().click(); + spyOn(vm, 'multiDeleteItems').and.returnValue(Promise.resolve()); - expect(vm.deleteItem).toHaveBeenCalledWith(firstImage); - expect(vm.itemToBeDeleted).toBeNull(); - done(); + Vue.nextTick(() => { + confirmationModal('.btn-danger').click(); + + expect(vm.itemsToBeDeleted).toEqual([]); + expect(vm.multiDeleteItems).toHaveBeenCalledWith({ + path: bulkDeletePath, + items: [firstImage.tag], + }); + done(); + }); }); }); }); @@ -65,4 +163,27 @@ describe('table registry', () => { expect(vm.$el.querySelector('.gl-pagination')).toBeDefined(); }); }); + + describe('modal content', () => { + it('should show the singular title and image name when deleting a single image', done => { + findDeleteBtnRow().click(); + + Vue.nextTick(() => { + expect(vm.modalTitle).toBe('Remove image'); + expect(vm.modalDescription).toContain(firstImage.tag); + done(); + }); + }); + + it('should show the plural title and image count when deleting more than one image', done => { + selectAllCheckboxes(); + vm.setModalDescription(); + + Vue.nextTick(() => { + expect(vm.modalTitle).toBe('Remove images'); + expect(vm.modalDescription).toContain('<b>2</b> images'); + done(); + }); + }); + }); }); diff --git a/spec/javascripts/registry/mock_data.js b/spec/javascripts/registry/mock_data.js index 22db203e77f..130ab298e89 100644 --- a/spec/javascripts/registry/mock_data.js +++ b/spec/javascripts/registry/mock_data.js @@ -108,6 +108,17 @@ export const repoPropsData = { destroyPath: 'path', canDelete: true, }, + { + tag: 'test-image', + revision: 'b969de599faea2b3d9b6605a8b0897261c571acaa36db1bdc7349b5775b4e0b4', + shortRevision: 'b969de599', + size: 19, + layers: 10, + location: 'location-2', + createdAt: 1505828744434, + destroyPath: 'path-2', + canDelete: true, + }, ], location: 'location', name: 'foo', diff --git a/spec/lib/banzai/filter/inline_metrics_filter_spec.rb b/spec/lib/banzai/filter/inline_metrics_filter_spec.rb index 542a9ced6d7..66bbcbf7292 100644 --- a/spec/lib/banzai/filter/inline_metrics_filter_spec.rb +++ b/spec/lib/banzai/filter/inline_metrics_filter_spec.rb @@ -12,7 +12,7 @@ describe Banzai::Filter::InlineMetricsFilter do let(:url) { 'https://foo.com' } it 'leaves regular non-metrics links unchanged' do - expect(doc.to_s).to eq input + expect(doc.to_s).to eq(input) end end @@ -21,7 +21,7 @@ describe Banzai::Filter::InlineMetricsFilter do let(:url) { urls.metrics_namespace_project_environment_url(*params) } it 'leaves the original link unchanged' do - expect(doc.at_css('a').to_s).to eq input + expect(doc.at_css('a').to_s).to eq(input) end it 'appends a metrics charts placeholder with dashboard url after metrics links' do @@ -29,7 +29,7 @@ describe Banzai::Filter::InlineMetricsFilter do expect(node).to be_present dashboard_url = urls.metrics_dashboard_namespace_project_environment_url(*params, embedded: true) - expect(node.attribute('data-dashboard-url').to_s).to eq dashboard_url + expect(node.attribute('data-dashboard-url').to_s).to eq(dashboard_url) end context 'when the metrics dashboard link is part of a paragraph' do @@ -37,9 +37,34 @@ describe Banzai::Filter::InlineMetricsFilter do let(:input) { %(<p>#{paragraph}</p>) } it 'appends the charts placeholder after the enclosing paragraph' do - expect(doc.at_css('p').to_s).to include paragraph + expect(doc.at_css('p').to_s).to include(paragraph) expect(doc.at_css('.js-render-metrics')).to be_present end end + + context 'with dashboard params specified' do + let(:params) do + [ + 'foo', + 'bar', + 12, + { + embedded: true, + dashboard: 'config/prometheus/common_metrics.yml', + group: 'System metrics (Kubernetes)', + title: 'Core Usage (Pod Average)', + y_label: 'Cores per Pod' + } + ] + end + + it 'appends a metrics charts placeholder with dashboard url after metrics links' do + node = doc.at_css('.js-render-metrics') + expect(node).to be_present + + dashboard_url = urls.metrics_dashboard_namespace_project_environment_url(*params) + expect(node.attribute('data-dashboard-url').to_s).to eq(dashboard_url) + end + end end end diff --git a/spec/lib/gitlab/danger/helper_spec.rb b/spec/lib/gitlab/danger/helper_spec.rb index f11f68ab3c2..2990594c538 100644 --- a/spec/lib/gitlab/danger/helper_spec.rb +++ b/spec/lib/gitlab/danger/helper_spec.rb @@ -101,13 +101,13 @@ describe Gitlab::Danger::Helper do describe '#changes_by_category' do it 'categorizes changed files' do - expect(fake_git).to receive(:added_files) { %w[foo foo.md foo.rb foo.js db/foo lib/gitlab/database/foo.rb qa/foo ee/changelogs/foo.yml] } + expect(fake_git).to receive(:added_files) { %w[foo foo.md foo.rb foo.js db/migrate/foo lib/gitlab/database/foo.rb qa/foo ee/changelogs/foo.yml] } allow(fake_git).to receive(:modified_files) { [] } allow(fake_git).to receive(:renamed_files) { [] } expect(helper.changes_by_category).to eq( backend: %w[foo.rb], - database: %w[db/foo lib/gitlab/database/foo.rb], + database: %w[db/migrate/foo lib/gitlab/database/foo.rb], frontend: %w[foo.js], none: %w[ee/changelogs/foo.yml foo.md], qa: %w[qa/foo], @@ -173,8 +173,13 @@ describe Gitlab::Danger::Helper do 'ee/FOO_VERSION' | :unknown - 'db/foo' | :database - 'ee/db/foo' | :database + 'db/schema.rb' | :database + 'db/migrate/foo' | :database + 'db/post_migrate/foo' | :database + 'ee/db/migrate/foo' | :database + 'ee/db/post_migrate/foo' | :database + 'ee/db/geo/migrate/foo' | :database + 'ee/db/geo/post_migrate/foo' | :database 'app/models/project_authorization.rb' | :database 'app/services/users/refresh_authorized_projects_service.rb' | :database 'lib/gitlab/background_migration.rb' | :database @@ -188,6 +193,9 @@ describe Gitlab::Danger::Helper do 'lib/gitlab/sql/foo' | :database 'rubocop/cop/migration/foo' | :database + 'db/fixtures/foo.rb' | :backend + 'ee/db/fixtures/foo.rb' | :backend + 'qa/foo' | :qa 'ee/qa/foo' | :qa diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index fddb5066d6f..3c6b17c10ec 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -242,6 +242,7 @@ project: - cluster_project - cluster_ingresses - creator +- cycle_analytics_stages - group - namespace - boards diff --git a/spec/lib/gitlab/metrics/dashboard/url_spec.rb b/spec/lib/gitlab/metrics/dashboard/url_spec.rb index 34bc6359414..e0dc6d98efc 100644 --- a/spec/lib/gitlab/metrics/dashboard/url_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/url_spec.rb @@ -9,14 +9,22 @@ describe Gitlab::Metrics::Dashboard::Url do end it 'matches a metrics dashboard link with named params' do - url = Gitlab::Routing.url_helpers.metrics_namespace_project_environment_url('foo', 'bar', 1, start: 123345456, anchor: 'title') + url = Gitlab::Routing.url_helpers.metrics_namespace_project_environment_url( + 'foo', + 'bar', + 1, + start: '2019-08-02T05:43:09.000Z', + dashboard: 'config/prometheus/common_metrics.yml', + group: 'awesome group', + anchor: 'title' + ) expected_params = { 'url' => url, 'namespace' => 'foo', 'project' => 'bar', 'environment' => '1', - 'query' => '?start=123345456', + 'query' => '?dashboard=config%2Fprometheus%2Fcommon_metrics.yml&group=awesome+group&start=2019-08-02T05%3A43%3A09.000Z', 'anchor' => '#title' } diff --git a/spec/lib/gitlab/project_template_spec.rb b/spec/lib/gitlab/project_template_spec.rb index 8b82ea7faa5..c7c82d07508 100644 --- a/spec/lib/gitlab/project_template_spec.rb +++ b/spec/lib/gitlab/project_template_spec.rb @@ -28,6 +28,18 @@ describe Gitlab::ProjectTemplate do end end + describe '#project_path' do + subject { described_class.new('name', 'title', 'description', 'https://gitlab.com/some/project/path').project_path } + + it { is_expected.to eq 'some/project/path' } + end + + describe '#uri_encoded_project_path' do + subject { described_class.new('name', 'title', 'description', 'https://gitlab.com/some/project/path').uri_encoded_project_path } + + it { is_expected.to eq 'some%2Fproject%2Fpath' } + end + describe '.find' do subject { described_class.find(query) } diff --git a/spec/models/analytics/cycle_analytics/project_stage_spec.rb b/spec/models/analytics/cycle_analytics/project_stage_spec.rb new file mode 100644 index 00000000000..4e3923e82b1 --- /dev/null +++ b/spec/models/analytics/cycle_analytics/project_stage_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Analytics::CycleAnalytics::ProjectStage do + describe 'associations' do + it { is_expected.to belong_to(:project) } + end +end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 53424204db7..d344a6d0f0d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -3015,9 +3015,6 @@ describe MergeRequest do subject { merge_request.rebase_in_progress? } it do - # Stub out the legacy gitaly implementation - allow(merge_request).to receive(:gitaly_rebase_in_progress?) { false } - allow(Gitlab::SidekiqStatus).to receive(:running?).with(rebase_jid) { jid_valid } merge_request.rebase_jid = rebase_jid @@ -3027,42 +3024,6 @@ describe MergeRequest do end end - describe '#gitaly_rebase_in_progress?' do - 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 - system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{rebase_path} master)) - end - - it 'returns true when there is a current rebase directory' do - expect(subject.rebase_in_progress?).to be_truthy - end - - it 'returns false when there is no rebase directory' do - FileUtils.rm_rf(rebase_path) - - expect(subject.rebase_in_progress?).to be_falsey - end - - it 'returns false when the rebase directory has expired' do - time = 20.minutes.ago.to_time - File.utime(time, time, rebase_path) - - expect(subject.rebase_in_progress?).to be_falsey - end - - it 'returns false when the source project has been removed' do - allow(subject).to receive(:source_project).and_return(nil) - - expect(subject.rebase_in_progress?).to be_falsey - end - end - describe '#allow_collaboration' do let(:merge_request) do build(:merge_request, source_branch: 'fixes', allow_collaboration: true) diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 2b9c3c43af9..972f26ac745 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -853,4 +853,64 @@ describe Namespace do it { is_expected.to be_falsy } end end + + describe '#emails_disabled?' do + context 'when not a subgroup' do + it 'returns false' do + group = create(:group, emails_disabled: false) + + expect(group.emails_disabled?).to be_falsey + end + + it 'returns true' do + group = create(:group, emails_disabled: true) + + expect(group.emails_disabled?).to be_truthy + end + end + + context 'when a subgroup' do + let(:grandparent) { create(:group) } + let(:parent) { create(:group, parent: grandparent) } + let(:group) { create(:group, parent: parent) } + + it 'returns false' do + expect(group.emails_disabled?).to be_falsey + end + + context 'when ancestor emails are disabled' do + it 'returns true' do + grandparent.update_attribute(:emails_disabled, true) + + expect(group.emails_disabled?).to be_truthy + end + end + end + + context 'when :emails_disabled feature flag is off' do + before do + stub_feature_flags(emails_disabled: false) + end + + context 'when not a subgroup' do + it 'returns false' do + group = create(:group, emails_disabled: true) + + expect(group.emails_disabled?).to be_falsey + end + end + + context 'when a subgroup and ancestor emails are disabled' do + let(:grandparent) { create(:group) } + let(:parent) { create(:group, parent: grandparent) } + let(:group) { create(:group, parent: parent) } + + it 'returns false' do + grandparent.update_attribute(:emails_disabled, true) + + expect(group.emails_disabled?).to be_falsey + end + end + end + end end diff --git a/spec/models/notification_recipient_spec.rb b/spec/models/notification_recipient_spec.rb index 4122736c148..2ba53818e54 100644 --- a/spec/models/notification_recipient_spec.rb +++ b/spec/models/notification_recipient_spec.rb @@ -9,6 +9,38 @@ describe NotificationRecipient do subject(:recipient) { described_class.new(user, :watch, target: target, project: project) } + describe '#notifiable?' do + let(:recipient) { described_class.new(user, :mention, target: target, project: project) } + + context 'when emails are disabled' do + it 'returns false if group disabled' do + expect(project.namespace).to receive(:emails_disabled?).and_return(true) + expect(recipient).to receive(:emails_disabled?).and_call_original + expect(recipient.notifiable?).to eq false + end + + it 'returns false if project disabled' do + expect(project).to receive(:emails_disabled?).and_return(true) + expect(recipient).to receive(:emails_disabled?).and_call_original + expect(recipient.notifiable?).to eq false + end + end + + context 'when emails are enabled' do + it 'returns true if group enabled' do + expect(project.namespace).to receive(:emails_disabled?).and_return(false) + expect(recipient).to receive(:emails_disabled?).and_call_original + expect(recipient.notifiable?).to eq true + end + + it 'returns true if project enabled' do + expect(project).to receive(:emails_disabled?).and_return(false) + expect(recipient).to receive(:emails_disabled?).and_call_original + expect(recipient.notifiable?).to eq true + end + end + end + describe '#has_access?' do before do allow(user).to receive(:can?).and_call_original diff --git a/spec/models/project_services/emails_on_push_service_spec.rb b/spec/models/project_services/emails_on_push_service_spec.rb index 0a58eb367e3..ffe241aa880 100644 --- a/spec/models/project_services/emails_on_push_service_spec.rb +++ b/spec/models/project_services/emails_on_push_service_spec.rb @@ -20,4 +20,24 @@ describe EmailsOnPushService do it { is_expected.not_to validate_presence_of(:recipients) } end end + + context 'project emails' do + let(:push_data) { { object_kind: 'push' } } + let(:project) { create(:project, :repository) } + let(:service) { create(:emails_on_push_service, project: project) } + + it 'does not send emails when disabled' do + expect(project).to receive(:emails_disabled?).and_return(true) + expect(EmailsOnPushWorker).not_to receive(:perform_async) + + service.execute(push_data) + end + + it 'does send emails when enabled' do + expect(project).to receive(:emails_disabled?).and_return(false) + expect(EmailsOnPushWorker).to receive(:perform_async) + + service.execute(push_data) + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2afe1253e29..ff9e94afc12 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -98,6 +98,7 @@ describe Project do it { is_expected.to have_many(:lfs_file_locks) } it { is_expected.to have_many(:project_deploy_tokens) } it { is_expected.to have_many(:deploy_tokens).through(:project_deploy_tokens) } + it { is_expected.to have_many(:cycle_analytics_stages) } it 'has an inverse relationship with merge requests' do expect(described_class.reflect_on_association(:merge_requests).has_inverse?).to eq(:target_project) @@ -2315,6 +2316,57 @@ describe Project do end end + describe '#emails_disabled?' do + let(:project) { create(:project, emails_disabled: false) } + + context 'emails disabled in group' do + it 'returns true' do + allow(project.namespace).to receive(:emails_disabled?) { true } + + expect(project.emails_disabled?).to be_truthy + end + end + + context 'emails enabled in group' do + before do + allow(project.namespace).to receive(:emails_disabled?) { false } + end + + it 'returns false' do + expect(project.emails_disabled?).to be_falsey + end + + it 'returns true' do + project.update_attribute(:emails_disabled, true) + + expect(project.emails_disabled?).to be_truthy + end + end + + context 'when :emails_disabled feature flag is off' do + before do + stub_feature_flags(emails_disabled: false) + end + + context 'emails disabled in group' do + it 'returns false' do + allow(project.namespace).to receive(:emails_disabled?) { true } + + expect(project.emails_disabled?).to be_falsey + end + end + + context 'emails enabled in group' do + it 'returns false' do + allow(project.namespace).to receive(:emails_disabled?) { false } + project.update_attribute(:emails_disabled, true) + + expect(project.emails_disabled?).to be_falsey + end + end + end + end + describe '#lfs_enabled?' do let(:project) { create(:project) } diff --git a/spec/requests/api/discussions_spec.rb b/spec/requests/api/discussions_spec.rb index ca1ffe3c524..ef09c6effbb 100644 --- a/spec/requests/api/discussions_spec.rb +++ b/spec/requests/api/discussions_spec.rb @@ -9,6 +9,61 @@ describe API::Discussions do project.add_developer(user) end + context 'with cross-reference system notes', :request_store do + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.project } + let(:new_merge_request) { create(:merge_request) } + let(:commit) { new_merge_request.project.commit } + let!(:note) { create(:system_note, noteable: merge_request, project: project, note: cross_reference) } + let!(:note_metadata) { create(:system_note_metadata, note: note, action: 'cross_reference') } + let(:cross_reference) { "test commit #{commit.to_reference(project)}" } + let(:pat) { create(:personal_access_token, user: user) } + + let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/discussions" } + + before do + project.add_developer(user) + new_merge_request.project.add_developer(user) + end + + it 'returns only the note that the user should see' do + hidden_merge_request = create(:merge_request) + new_cross_reference = "test commit #{hidden_merge_request.project.commit}" + new_note = create(:system_note, noteable: merge_request, project: project, note: new_cross_reference) + create(:system_note_metadata, note: new_note, action: 'cross_reference') + + get api(url, user, personal_access_token: pat) + expect(response).to have_gitlab_http_status(200) + expect(json_response.count).to eq(1) + expect(json_response.first['notes'].count).to eq(1) + + parsed_note = json_response.first['notes'].first + expect(parsed_note['id']).to eq(note.id) + expect(parsed_note['body']).to eq(cross_reference) + expect(parsed_note['system']).to be true + end + + it 'avoids Git calls and N+1 SQL queries' do + expect_any_instance_of(Repository).not_to receive(:find_commit).with(commit.id) + + control = ActiveRecord::QueryRecorder.new do + get api(url, user, personal_access_token: pat) + end + + expect(response).to have_gitlab_http_status(200) + + RequestStore.clear! + + new_note = create(:system_note, noteable: merge_request, project: project, note: cross_reference) + create(:system_note_metadata, note: new_note, action: 'cross_reference') + + RequestStore.clear! + + expect { get api(url, user, personal_access_token: pat) }.not_to exceed_query_limit(control) + expect(response).to have_gitlab_http_status(200) + end + end + context 'when noteable is an Issue' do let!(:issue) { create(:issue, project: project, author: user) } let!(:issue_note) { create(:discussion_note_on_issue, noteable: issue, project: project, author: user) } diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb index 5d4576139f7..12e9c2b2f3a 100644 --- a/spec/services/groups/update_service_spec.rb +++ b/spec/services/groups/update_service_spec.rb @@ -86,6 +86,7 @@ describe Groups::UpdateService do context "unauthorized visibility_level validation" do let!(:service) { described_class.new(internal_group, user, visibility_level: 99) } + before do internal_group.add_user(user, Gitlab::Access::MAINTAINER) end @@ -96,6 +97,20 @@ describe Groups::UpdateService do end end + context 'when updating #emails_disabled' do + let(:service) { described_class.new(internal_group, user, emails_disabled: true) } + + it 'updates the attribute' do + internal_group.add_user(user, Gitlab::Access::OWNER) + + expect { service.execute }.to change { internal_group.emails_disabled }.to(true) + end + + it 'does not update when not group owner' do + expect { service.execute }.not_to change { internal_group.emails_disabled } + end + end + context 'rename group' do let!(:service) { described_class.new(internal_group, user, path: SecureRandom.hex) } diff --git a/spec/services/merge_requests/rebase_service_spec.rb b/spec/services/merge_requests/rebase_service_spec.rb index ee9caaf2f47..7b8c94c86fe 100644 --- a/spec/services/merge_requests/rebase_service_spec.rb +++ b/spec/services/merge_requests/rebase_service_spec.rb @@ -25,7 +25,7 @@ describe MergeRequests::RebaseService do describe '#execute' do context 'when another rebase is already in progress' do before do - allow(merge_request).to receive(:gitaly_rebase_in_progress?).and_return(true) + allow(repository).to receive(:rebase_in_progress?).with(merge_request.id).and_return(true) end it 'saves the error message' do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 1dcade1de0d..d925aa2b6c3 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -240,45 +240,50 @@ describe NotificationService, :mailer do end describe '#new_note' do - it do - add_users(project) - add_user_subscriptions(issue) - reset_delivered_emails! + context do + before do + add_users(project) + add_user_subscriptions(issue) + reset_delivered_emails! + end - expect(SentNotification).to receive(:record).with(issue, any_args).exactly(10).times + it do + expect(SentNotification).to receive(:record).with(issue, any_args).exactly(10).times - notification.new_note(note) + notification.new_note(note) - should_email(@u_watcher) - should_email(note.noteable.author) - should_email(note.noteable.assignees.first) - should_email(@u_custom_global) - should_email(@u_mentioned) - should_email(@subscriber) - should_email(@watcher_and_subscriber) - should_email(@subscribed_participant) - should_email(@u_custom_off) - should_email(@unsubscribed_mentioned) - should_not_email(@u_guest_custom) - should_not_email(@u_guest_watcher) - should_not_email(note.author) - should_not_email(@u_participating) - should_not_email(@u_disabled) - should_not_email(@unsubscriber) - should_not_email(@u_outsider_mentioned) - should_not_email(@u_lazy_participant) - end + should_email(@u_watcher) + should_email(note.noteable.author) + should_email(note.noteable.assignees.first) + should_email(@u_custom_global) + should_email(@u_mentioned) + should_email(@subscriber) + should_email(@watcher_and_subscriber) + should_email(@subscribed_participant) + should_email(@u_custom_off) + should_email(@unsubscribed_mentioned) + should_not_email(@u_guest_custom) + should_not_email(@u_guest_watcher) + should_not_email(note.author) + should_not_email(@u_participating) + should_not_email(@u_disabled) + should_not_email(@unsubscriber) + should_not_email(@u_outsider_mentioned) + should_not_email(@u_lazy_participant) + end - it "emails the note author if they've opted into notifications about their activity" do - add_users(project) - add_user_subscriptions(issue) - reset_delivered_emails! + it "emails the note author if they've opted into notifications about their activity" do + note.author.notified_of_own_activity = true - note.author.notified_of_own_activity = true + notification.new_note(note) - notification.new_note(note) + should_email(note.author) + end - should_email(note.author) + it_behaves_like 'project emails are disabled' do + let(:notification_target) { note } + let(:notification_trigger) { notification.new_note(note) } + end end it 'filters out "mentioned in" notes' do @@ -337,6 +342,11 @@ describe NotificationService, :mailer do it_behaves_like 'new note notifications' + it_behaves_like 'project emails are disabled' do + let(:notification_target) { note } + let(:notification_trigger) { notification.new_note(note) } + end + context 'which is a subgroup' do let!(:parent) { create(:group) } let!(:group) { create(:group, parent: parent) } @@ -472,6 +482,11 @@ describe NotificationService, :mailer do expect(Notify).not_to receive(:note_issue_email) notification.new_note(mentioned_note) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { note } + let(:notification_trigger) { notification.new_note(note) } + end end end @@ -619,6 +634,11 @@ describe NotificationService, :mailer do notification.new_note(note) should_not_email(@u_committer) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { note } + let(:notification_trigger) { notification.new_note(note) } + end end end @@ -645,6 +665,11 @@ describe NotificationService, :mailer do .to contain_exactly(*merge_request.assignees.pluck(:id), merge_request.author.id, @u_watcher.id) expect(SentNotification.last.in_reply_to_discussion_id).to eq(note.discussion_id) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { note } + let(:notification_trigger) { notification.new_note(note) } + end end end end @@ -819,6 +844,11 @@ describe NotificationService, :mailer do should_email(user_4) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.new_issue(issue, @u_disabled) } + end + context 'confidential issues' do let(:author) { create(:user) } let(:assignee) { create(:user) } @@ -861,6 +891,11 @@ describe NotificationService, :mailer do let(:mentionable) { issue } include_examples 'notifications for new mentions' + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { send_notifications(@u_watcher, @u_participant_mentioned, @u_custom_global, @u_mentioned) } + end end describe '#reassigned_issue' do @@ -969,6 +1004,11 @@ describe NotificationService, :mailer do let(:issuable) { issue } let(:notification_trigger) { notification.reassigned_issue(issue, @u_disabled, [assignee]) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.reassigned_issue(issue, @u_disabled, [assignee]) } + end end describe '#relabeled_issue' do @@ -1028,6 +1068,11 @@ describe NotificationService, :mailer do should_email(subscriber_to_both) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.relabeled_issue(issue, [group_label_2, label_2], @u_disabled) } + end + context 'confidential issues' do let(:author) { create(:user) } let(:assignee) { create(:user) } @@ -1065,12 +1110,19 @@ describe NotificationService, :mailer do end describe '#removed_milestone_issue' do - it_behaves_like 'altered milestone notification on issue' do + context do let(:milestone) { create(:milestone, project: project, issues: [issue]) } let!(:subscriber_to_new_milestone) { create(:user) { |u| issue.toggle_subscription(u, project) } } - before do - notification.removed_milestone_issue(issue, issue.author) + it_behaves_like 'altered milestone notification on issue' do + before do + notification.removed_milestone_issue(issue, issue.author) + end + end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.removed_milestone_issue(issue, issue.author) } end end @@ -1110,12 +1162,19 @@ describe NotificationService, :mailer do end describe '#changed_milestone_issue' do - it_behaves_like 'altered milestone notification on issue' do + context do let(:new_milestone) { create(:milestone, project: project, issues: [issue]) } let!(:subscriber_to_new_milestone) { create(:user) { |u| issue.toggle_subscription(u, project) } } - before do - notification.changed_milestone_issue(issue, new_milestone, issue.author) + it_behaves_like 'altered milestone notification on issue' do + before do + notification.changed_milestone_issue(issue, new_milestone, issue.author) + end + end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.changed_milestone_issue(issue, new_milestone, issue.author) } end end @@ -1183,6 +1242,11 @@ describe NotificationService, :mailer do let(:issuable) { issue } let(:notification_trigger) { notification.close_issue(issue, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.close_issue(issue, @u_disabled) } + end end describe '#reopen_issue' do @@ -1214,6 +1278,11 @@ describe NotificationService, :mailer do let(:issuable) { issue } let(:notification_trigger) { notification.reopen_issue(issue, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.reopen_issue(issue, @u_disabled) } + end end describe '#issue_moved' do @@ -1240,6 +1309,11 @@ describe NotificationService, :mailer do let(:issuable) { issue } let(:notification_trigger) { notification.issue_moved(issue, new_issue, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.issue_moved(issue, new_issue, @u_disabled) } + end end describe '#issue_due' do @@ -1280,6 +1354,11 @@ describe NotificationService, :mailer do let(:issuable) { issue } let(:notification_trigger) { notification.issue_due(issue) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.issue_due(issue) } + end end end @@ -1374,6 +1453,11 @@ describe NotificationService, :mailer do should_email(user_4) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.new_merge_request(merge_request, @u_disabled) } + end + context 'participating' do it_should_behave_like 'participating by assignee notification' do let(:participant) { create(:user, username: 'user-participant')} @@ -1406,6 +1490,11 @@ describe NotificationService, :mailer do let(:mentionable) { merge_request } include_examples 'notifications for new mentions' + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { send_notifications(@u_watcher, @u_participant_mentioned, @u_custom_global, @u_mentioned) } + end end describe '#reassigned_merge_request' do @@ -1449,6 +1538,11 @@ describe NotificationService, :mailer do let(:issuable) { merge_request } let(:notification_trigger) { notification.reassigned_merge_request(merge_request, current_user, [assignee]) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.reassigned_merge_request(merge_request, current_user, [assignee]) } + end end describe '#push_to_merge_request' do @@ -1479,6 +1573,11 @@ describe NotificationService, :mailer do let(:issuable) { merge_request } let(:notification_trigger) { notification.push_to_merge_request(merge_request, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.push_to_merge_request(merge_request, @u_disabled) } + end end describe '#relabel_merge_request' do @@ -1512,28 +1611,43 @@ describe NotificationService, :mailer do should_not_email(@u_participating) should_not_email(@u_lazy_participant) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.relabeled_merge_request(merge_request, [group_label_2, label_2], @u_disabled) } + end end describe '#removed_milestone_merge_request' do - it_behaves_like 'altered milestone notification on merge request' do - let(:milestone) { create(:milestone, project: project, merge_requests: [merge_request]) } - let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } } + let(:milestone) { create(:milestone, project: project, merge_requests: [merge_request]) } + let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } } + it_behaves_like 'altered milestone notification on merge request' do before do notification.removed_milestone_merge_request(merge_request, merge_request.author) end end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.removed_milestone_merge_request(merge_request, merge_request.author) } + end end describe '#changed_milestone_merge_request' do - it_behaves_like 'altered milestone notification on merge request' do - let(:new_milestone) { create(:milestone, project: project, merge_requests: [merge_request]) } - let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } } + let(:new_milestone) { create(:milestone, project: project, merge_requests: [merge_request]) } + let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } } + it_behaves_like 'altered milestone notification on merge request' do before do notification.changed_milestone_merge_request(merge_request, new_milestone, merge_request.author) end end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.changed_milestone_merge_request(merge_request, new_milestone, merge_request.author) } + end end describe '#merge_request_unmergeable' do @@ -1544,6 +1658,11 @@ describe NotificationService, :mailer do expect(email_recipients.size).to eq(1) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.merge_request_unmergeable(merge_request) } + end + describe 'when merge_when_pipeline_succeeds is true' do before do merge_request.update( @@ -1590,6 +1709,11 @@ describe NotificationService, :mailer do let(:issuable) { merge_request } let(:notification_trigger) { notification.close_mr(merge_request, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.close_mr(merge_request, @u_disabled) } + end end describe '#merged_merge_request' do @@ -1642,6 +1766,11 @@ describe NotificationService, :mailer do let(:issuable) { merge_request } let(:notification_trigger) { notification.merge_mr(merge_request, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.merge_mr(merge_request, @u_disabled) } + end end describe '#reopen_merge_request' do @@ -1672,6 +1801,11 @@ describe NotificationService, :mailer do let(:issuable) { merge_request } let(:notification_trigger) { notification.reopen_mr(merge_request, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.reopen_mr(merge_request, @u_disabled) } + end end describe "#resolve_all_discussions" do @@ -1695,6 +1829,11 @@ describe NotificationService, :mailer do let(:issuable) { merge_request } let(:notification_trigger) { notification.resolve_all_discussions(merge_request, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.resolve_all_discussions(merge_request, @u_disabled) } + end end end @@ -1719,6 +1858,11 @@ describe NotificationService, :mailer do should_not_email(@u_disabled) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.project_was_moved(project, "gitlab/gitlab") } + end + context 'users not having access to the new location' do it 'does not send email' do old_user = create(:user) @@ -1762,6 +1906,11 @@ describe NotificationService, :mailer do should_only_email(@u_participating) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.project_exported(project, @u_participating) } + end end describe '#project_not_exported' do @@ -1770,6 +1919,11 @@ describe NotificationService, :mailer do should_only_email(@u_participating) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.project_not_exported(project, @u_participating, ['error']) } + end end end end @@ -1800,6 +1954,11 @@ describe NotificationService, :mailer do should_email(maintainer) should_not_email(developer) end + + it_behaves_like 'group emails are disabled' do + let(:notification_target) { group } + let(:notification_trigger) { group.request_access(added_user) } + end end describe '#decline_group_invite' do @@ -1839,6 +1998,11 @@ describe NotificationService, :mailer do should_not_email_anyone end end + + it_behaves_like 'group emails are disabled' do + let(:notification_target) { group } + let(:notification_trigger) { group.add_guest(added_user) } + end end end @@ -1859,6 +2023,11 @@ describe NotificationService, :mailer do should_only_email(project.owner) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { project.request_access(added_user) } + end end context 'for a project in a group' do @@ -1878,7 +2047,7 @@ describe NotificationService, :mailer do end end - describe '#decline_group_invite' do + describe '#decline_project_invite' do let(:member) { create(:user) } before do @@ -1900,6 +2069,11 @@ describe NotificationService, :mailer do should_only_email(added_user) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { create_member! } + end + context 'when notifications are disabled' do before do create_global_setting_for(added_user, :disabled) @@ -2071,6 +2245,11 @@ describe NotificationService, :mailer do should_only_email(u_custom_notification_enabled, kind: :bcc) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { pipeline } + let(:notification_trigger) { notification.pipeline_finished(pipeline) } + end + context 'when the creator has group notification email set' do let(:group_notification_email) { 'user+group@example.com' } @@ -2100,6 +2279,11 @@ describe NotificationService, :mailer do should_only_email(u_member, kind: :bcc) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { pipeline } + let(:notification_trigger) { notification.pipeline_finished(pipeline) } + end + context 'when the creator has group notification email set' do let(:group_notification_email) { 'user+group@example.com' } @@ -2215,6 +2399,11 @@ describe NotificationService, :mailer do should_only_email(u_maintainer1, u_maintainer2, u_owner) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { domain } + let(:notification_trigger) { notify! } + end + it 'emails nobody if the project is missing' do domain.project = nil @@ -2224,30 +2413,6 @@ describe NotificationService, :mailer do end end end - - describe '#pages_domain_verification_failed' do - it 'emails current watching maintainers' do - notification.pages_domain_verification_failed(domain) - - should_only_email(u_maintainer1, u_maintainer2, u_owner) - end - end - - describe '#pages_domain_enabled' do - it 'emails current watching maintainers' do - notification.pages_domain_enabled(domain) - - should_only_email(u_maintainer1, u_maintainer2, u_owner) - end - end - - describe '#pages_domain_disabled' do - it 'emails current watching maintainers' do - notification.pages_domain_disabled(domain) - - should_only_email(u_maintainer1, u_maintainer2, u_owner) - end - end end context 'Auto DevOps notifications' do @@ -2266,6 +2431,11 @@ describe NotificationService, :mailer do should_email(owner, times: 1) # Once for the disable pipeline. should_email(pipeline_user, times: 2) # Once for the new permission, once for the disable. end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.autodevops_disabled(pipeline, [owner.email, pipeline_user.email]) } + end end end @@ -2279,6 +2449,11 @@ describe NotificationService, :mailer do should_email(user) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.repository_cleanup_success(project, user) } + end end describe '#repository_cleanup_failure' do @@ -2287,6 +2462,11 @@ describe NotificationService, :mailer do should_email(user) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.repository_cleanup_failure(project, user, 'Some error') } + end end end @@ -2320,6 +2500,11 @@ describe NotificationService, :mailer do should_only_email(u_maintainer1, u_maintainer2, u_owner) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.remote_mirror_update_failed(remote_mirror) } + end end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 82010dd283c..31bd0f0f836 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -369,9 +369,28 @@ describe Projects::UpdateService do end end + context 'when updating #emails_disabled' do + it 'updates the attribute for the project owner' do + expect { update_project(project, user, emails_disabled: true) } + .to change { project.emails_disabled } + .to(true) + end + + it 'does not update when not project owner' do + maintainer = create(:user) + project.add_user(maintainer, :maintainer) + + expect { update_project(project, maintainer, emails_disabled: true) } + .not_to change { project.emails_disabled } + end + end + context 'with external authorization enabled' do before do enable_external_authorization_service_check + + allow(::Gitlab::ExternalAuthorization) + .to receive(:access_allowed?).with(user, 'default_label', project.full_path).and_call_original end it 'does not save the project with an error if the service denies access' do @@ -402,8 +421,7 @@ describe Projects::UpdateService do end it 'does not check the label when it does not change' do - expect(::Gitlab::ExternalAuthorization) - .not_to receive(:access_allowed?) + expect(::Gitlab::ExternalAuthorization).to receive(:access_allowed?).once update_project(project, user, { name: 'New name' }) end diff --git a/spec/support/helpers/email_helpers.rb b/spec/support/helpers/email_helpers.rb index 83ba654fab3..024340310a1 100644 --- a/spec/support/helpers/email_helpers.rb +++ b/spec/support/helpers/email_helpers.rb @@ -31,6 +31,10 @@ module EmailHelpers expect(ActionMailer::Base.deliveries).to be_empty end + def should_email_anyone + expect(ActionMailer::Base.deliveries).not_to be_empty + end + def email_recipients(kind: :to) ActionMailer::Base.deliveries.flat_map(&kind) end diff --git a/spec/support/shared_examples/services/notification_service_shared_examples.rb b/spec/support/shared_examples/services/notification_service_shared_examples.rb new file mode 100644 index 00000000000..dd338ea47c7 --- /dev/null +++ b/spec/support/shared_examples/services/notification_service_shared_examples.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +# Note that we actually update the attribute on the target_project/group, rather than +# using `allow`. This is because there are some specs where, based on how the notification +# is done, using an `allow` doesn't change the correct object. +shared_examples 'project emails are disabled' do + let(:target_project) { notification_target.is_a?(Project) ? notification_target : notification_target.project } + + before do + reset_delivered_emails! + target_project.clear_memoization(:emails_disabled) + end + + it 'sends no emails with project emails disabled' do + target_project.update_attribute(:emails_disabled, true) + + notification_trigger + + should_not_email_anyone + end + + it 'sends emails to someone' do + target_project.update_attribute(:emails_disabled, false) + + notification_trigger + + should_email_anyone + end +end + +shared_examples 'group emails are disabled' do + let(:target_group) { notification_target.is_a?(Group) ? notification_target : notification_target.project.group } + + before do + reset_delivered_emails! + target_group.clear_memoization(:emails_disabled) + end + + it 'sends no emails with group emails disabled' do + target_group.update_attribute(:emails_disabled, true) + + notification_trigger + + should_not_email_anyone + end + + it 'sends emails to someone' do + target_group.update_attribute(:emails_disabled, false) + + notification_trigger + + should_email_anyone + end +end diff --git a/spec/tasks/gitlab/update_templates_rake_spec.rb b/spec/tasks/gitlab/update_templates_rake_spec.rb index 7b17549b8c7..14b4ad5e3d8 100644 --- a/spec/tasks/gitlab/update_templates_rake_spec.rb +++ b/spec/tasks/gitlab/update_templates_rake_spec.rb @@ -8,9 +8,18 @@ describe 'gitlab:update_project_templates rake task' do before do Rake.application.rake_require 'tasks/gitlab/update_templates' create(:admin) + allow(Gitlab::ProjectTemplate) .to receive(:archive_directory) .and_return(Pathname.new(tmpdir)) + + # Gitlab::HTTP resolves the domain to an IP prior to WebMock taking effect, hence the wildcard + stub_request(:get, %r{^https://.*/api/v4/projects/gitlab-org%2Fproject-templates%2Frails/repository/commits\?page=1&per_page=1}) + .to_return( + status: 200, + body: [{ id: '67812735b83cb42710f22dc98d73d42c8bf4d907' }].to_json, + headers: { 'Content-Type' => 'application/json' } + ) end after do diff --git a/yarn.lock b/yarn.lock index ed1f06523c0..a295039ec54 100644 --- a/yarn.lock +++ b/yarn.lock @@ -998,10 +998,10 @@ dependencies: vue-eslint-parser "^6.0.4" -"@gitlab/svgs@^1.67.0": - version "1.67.0" - resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.67.0.tgz#c7b94eca13b99fd3aaa737fb6dcc0abc41d3c579" - integrity sha512-hJOmWEs6RkjzyKkb1vc9wwKGZIBIP0coHkxu/KgOoxhBVudpGk4CH7xJ6UuB2TKpb0SEh5CC1CzRZfBYaFhsaA== +"@gitlab/svgs@^1.68.0": + version "1.68.0" + resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.68.0.tgz#d631bd84ea7907592240d8417e82ba66d6a54c0c" + integrity sha512-3JmIq0bHg4InjLooM+kQFPfg3d7B1Pye67pN9+12kZXIa0nRGuwKEq3WSbcS+ACdg5jcVPNPYqStItEO4teHdw== "@gitlab/ui@5.15.0": version "5.15.0" |