diff options
69 files changed, 1431 insertions, 246 deletions
diff --git a/app/assets/javascripts/ide/components/commit_sidebar/empty_state.vue b/app/assets/javascripts/ide/components/commit_sidebar/empty_state.vue index a23bae8e4c7..a13ca0cd138 100644 --- a/app/assets/javascripts/ide/components/commit_sidebar/empty_state.vue +++ b/app/assets/javascripts/ide/components/commit_sidebar/empty_state.vue @@ -9,10 +9,7 @@ export default { </script> <template> - <div - v-if="!lastCommitMsg" - class="multi-file-commit-panel-section ide-commit-empty-state js-empty-state" - > + <div v-if="!lastCommitMsg" class="multi-file-commit-panel-section ide-commit-empty-state"> <div class="ide-commit-empty-state-container"> <div class="svg-content svg-80"><img :src="noChangesStateSvgPath" /></div> <div class="append-right-default prepend-left-default"> diff --git a/app/assets/javascripts/ide/components/repo_commit_section.vue b/app/assets/javascripts/ide/components/repo_commit_section.vue index 530fba49df2..b6c839269e3 100644 --- a/app/assets/javascripts/ide/components/repo_commit_section.vue +++ b/app/assets/javascripts/ide/components/repo_commit_section.vue @@ -14,12 +14,12 @@ export default { tooltip, }, computed: { - ...mapState(['changedFiles', 'stagedFiles', 'lastCommitMsg', 'unusedSeal']), + ...mapState(['changedFiles', 'stagedFiles', 'lastCommitMsg']), ...mapState('commit', ['commitMessage', 'submitCommitLoading']), ...mapGetters(['lastOpenedFile', 'hasChanges', 'someUncommittedChanges', 'activeFile']), ...mapGetters('commit', ['discardDraftButtonDisabled']), showStageUnstageArea() { - return Boolean(this.someUncommittedChanges || this.lastCommitMsg || !this.unusedSeal); + return Boolean(this.someUncommittedChanges || this.lastCommitMsg); }, activeFileKey() { return this.activeFile ? this.activeFile.key : null; @@ -67,6 +67,6 @@ export default { icon-name="unstaged" /> </template> - <empty-state v-if="unusedSeal" /> + <empty-state v-else /> </div> </template> diff --git a/app/assets/javascripts/ide/stores/mutations.js b/app/assets/javascripts/ide/stores/mutations.js index 12ac10df206..0432d87fd76 100644 --- a/app/assets/javascripts/ide/stores/mutations.js +++ b/app/assets/javascripts/ide/stores/mutations.js @@ -207,8 +207,6 @@ export default { state.changedFiles = state.changedFiles.concat(entry); } } - - state.unusedSeal = false; }, [types.RENAME_ENTRY](state, { path, name, parentPath }) { const oldEntry = state.entries[path]; diff --git a/app/assets/javascripts/ide/stores/mutations/file.js b/app/assets/javascripts/ide/stores/mutations/file.js index 5c5920a3027..313fa1fe029 100644 --- a/app/assets/javascripts/ide/stores/mutations/file.js +++ b/app/assets/javascripts/ide/stores/mutations/file.js @@ -153,13 +153,11 @@ export default { [types.ADD_FILE_TO_CHANGED](state, path) { Object.assign(state, { changedFiles: state.changedFiles.concat(state.entries[path]), - unusedSeal: false, }); }, [types.REMOVE_FILE_FROM_CHANGED](state, path) { Object.assign(state, { changedFiles: state.changedFiles.filter(f => f.path !== path), - unusedSeal: false, }); }, [types.STAGE_CHANGE](state, { path, diffInfo }) { @@ -175,7 +173,6 @@ export default { deleted: diffInfo.deleted, }), }), - unusedSeal: false, }); if (stagedFile) { diff --git a/app/assets/javascripts/ide/stores/state.js b/app/assets/javascripts/ide/stores/state.js index 0c95c22e8f8..8f3bc68d15d 100644 --- a/app/assets/javascripts/ide/stores/state.js +++ b/app/assets/javascripts/ide/stores/state.js @@ -20,7 +20,6 @@ export default () => ({ viewer: viewerTypes.edit, delayViewerUpdated: false, currentActivityView: leftSidebarViews.edit.name, - unusedSeal: true, fileFindVisible: false, links: {}, errorMessage: null, diff --git a/app/assets/javascripts/monitoring/stores/actions.js b/app/assets/javascripts/monitoring/stores/actions.js index be796e80ba9..a9b8b1db922 100644 --- a/app/assets/javascripts/monitoring/stores/actions.js +++ b/app/assets/javascripts/monitoring/stores/actions.js @@ -218,13 +218,16 @@ export const fetchPrometheusMetric = ( { commit, state, getters }, { metric, defaultQueryParams }, ) => { - const queryParams = { ...defaultQueryParams }; + let queryParams = { ...defaultQueryParams }; if (metric.step) { queryParams.step = metric.step; } if (Object.keys(state.variables).length > 0) { - queryParams.variables = getters.getCustomVariablesArray; + queryParams = { + ...queryParams, + ...getters.getCustomVariablesParams, + }; } commit(types.REQUEST_METRIC_RESULT, { metricId: metric.metricId }); diff --git a/app/assets/javascripts/monitoring/stores/getters.js b/app/assets/javascripts/monitoring/stores/getters.js index f3b1e5a7dde..2c721e8d5a2 100644 --- a/app/assets/javascripts/monitoring/stores/getters.js +++ b/app/assets/javascripts/monitoring/stores/getters.js @@ -1,5 +1,5 @@ -import { flatMap } from 'lodash'; import { NOT_IN_DB_PREFIX } from '../constants'; +import { addPrefixToCustomVariableParams } from './utils'; const metricsIdsInPanel = panel => panel.metrics.filter(metric => metric.metricId && metric.result).map(metric => metric.metricId); @@ -116,13 +116,27 @@ export const filteredEnvironments = state => * Maps an variables object to an array along with stripping * the variable prefix. * + * This method outputs an object in the below format + * + * { + * variables[key1]=value1, + * variables[key2]=value2, + * } + * + * This is done so that the backend can identify the custom + * user-defined variables coming through the URL and differentiate + * from other variables used for Prometheus API endpoint. + * * @param {Object} variables - Custom variables provided by the user * @returns {Array} The custom variables array to be send to the API - * in the format of [variable1, variable1_value] + * in the format of {variables[key1]=value1, variables[key2]=value2} */ -export const getCustomVariablesArray = state => - flatMap(state.variables, (variable, key) => [key, variable.value]); +export const getCustomVariablesParams = state => + Object.keys(state.variables).reduce((acc, variable) => { + acc[addPrefixToCustomVariableParams(variable)] = state.variables[variable]?.value; + return acc; + }, {}); // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/monitoring/stores/utils.js b/app/assets/javascripts/monitoring/stores/utils.js index 67a8a46a098..060b2e7013e 100644 --- a/app/assets/javascripts/monitoring/stores/utils.js +++ b/app/assets/javascripts/monitoring/stores/utils.js @@ -249,3 +249,19 @@ export const normalizeQueryResult = timeSeries => { return normalizedResult; }; + +/** + * Custom variables defined in the dashboard yml file are + * eventually passed over the wire to the backend Prometheus + * API proxy. + * + * This method adds a prefix to the URL param keys so that + * the backend can differential these variables from the other + * variables. + * + * This is currently only used by getters/getCustomVariablesParams + * + * @param {String} key Variable key that needs to be prefixed + * @returns {String} + */ +export const addPrefixToCustomVariableParams = key => `variables[${key}]`; diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index ffbccbb01f2..c7da029d2f3 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -14,6 +14,7 @@ class RegistrationsController < Devise::RegistrationsController before_action :ensure_terms_accepted, if: -> { action_name == 'create' && Gitlab::CurrentSettings.current_application_settings.enforce_terms? } before_action :load_recaptcha, only: :new + before_action :authenticate_user!, only: :experience_level def new if experiment_enabled?(:signup_flow) @@ -57,6 +58,10 @@ class RegistrationsController < Devise::RegistrationsController return redirect_to path_for_signed_in_user(current_user) if current_user.role.present? && !current_user.setup_for_company.nil? end + def experience_level + return access_denied! unless experiment_enabled?(:onboarding_issues) + end + def update_registration user_params = params.require(:user).permit(:role, :setup_for_company) result = ::Users::SignupService.new(current_user, user_params).execute diff --git a/app/helpers/milestones_helper.rb b/app/helpers/timeboxes_helper.rb index df1ee54c5ac..87ea22d8f83 100644 --- a/app/helpers/milestones_helper.rb +++ b/app/helpers/timeboxes_helper.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -module MilestonesHelper +module TimeboxesHelper include EntityDateHelper include Gitlab::Utils::StrongMemoize @@ -209,23 +209,24 @@ module MilestonesHelper end end - def milestone_date_range(milestone) - if milestone.start_date && milestone.due_date - "#{milestone.start_date.to_s(:medium)}–#{milestone.due_date.to_s(:medium)}" - elsif milestone.due_date - if milestone.due_date.past? - _("expired on %{milestone_due_date}") % { milestone_due_date: milestone.due_date.strftime('%b %-d, %Y') } + def timebox_date_range(timebox) + if timebox.start_date && timebox.due_date + "#{timebox.start_date.to_s(:medium)}–#{timebox.due_date.to_s(:medium)}" + elsif timebox.due_date + if timebox.due_date.past? + _("expired on %{timebox_due_date}") % { timebox_due_date: timebox.due_date.to_s(:medium) } else - _("expires on %{milestone_due_date}") % { milestone_due_date: milestone.due_date.strftime('%b %-d, %Y') } + _("expires on %{timebox_due_date}") % { timebox_due_date: timebox.due_date.to_s(:medium) } end - elsif milestone.start_date - if milestone.start_date.past? - _("started on %{milestone_start_date}") % { milestone_start_date: milestone.start_date.strftime('%b %-d, %Y') } + elsif timebox.start_date + if timebox.start_date.past? + _("started on %{timebox_start_date}") % { timebox_start_date: timebox.start_date.to_s(:medium) } else - _("starts on %{milestone_start_date}") % { milestone_start_date: milestone.start_date.strftime('%b %-d, %Y') } + _("starts on %{timebox_start_date}") % { timebox_start_date: timebox.start_date.to_s(:medium) } end end end + alias_method :milestone_date_range, :timebox_date_range def milestone_tab_path(milestone, tab) if milestone.global_milestone? @@ -306,4 +307,4 @@ module MilestonesHelper end end -MilestonesHelper.prepend_if_ee('EE::MilestonesHelper') +TimeboxesHelper.prepend_if_ee('EE::TimeboxesHelper') diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb index d4d93ab9795..bcf60bea0e0 100644 --- a/app/mailers/emails/issues.rb +++ b/app/mailers/emails/issues.rb @@ -126,3 +126,5 @@ module Emails end end end + +Emails::Issues.prepend_if_ee('EE::Emails::Issues') diff --git a/app/mailers/emails/reviews.rb b/app/mailers/emails/reviews.rb new file mode 100644 index 00000000000..ddb9e161a80 --- /dev/null +++ b/app/mailers/emails/reviews.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Emails + module Reviews + def new_review_email(recipient_id, review_id) + setup_review_email(review_id, recipient_id) + + mail_answer_thread(@merge_request, review_thread_options(recipient_id)) + end + + private + + def review_thread_options(recipient_id) + { + from: sender(@author.id), + to: User.find(recipient_id).notification_email_for(@merge_request.target_project.group), + subject: subject("#{@merge_request.title} (#{@merge_request.to_reference})") + } + end + + def setup_review_email(review_id, recipient_id) + review = Review.find_by_id(review_id) + + @notes = review.notes + @author = review.author + @author_name = review.author_name + @project = review.project + @merge_request = review.merge_request + @target_url = project_merge_request_url(@project, @merge_request) + @sent_notification = SentNotification.record(@merge_request, recipient_id, reply_key) + end + end +end diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index d9483bab543..2cf72d40635 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -18,8 +18,9 @@ class Notify < ApplicationMailer include Emails::RemoteMirrors include Emails::Releases include Emails::Groups + include Emails::Reviews - helper MilestonesHelper + helper TimeboxesHelper helper MergeRequestsHelper helper DiffHelper helper BlobHelper diff --git a/app/models/concerns/limitable.rb b/app/models/concerns/limitable.rb index f320f54bb82..3cb0bd85936 100644 --- a/app/models/concerns/limitable.rb +++ b/app/models/concerns/limitable.rb @@ -2,6 +2,7 @@ module Limitable extend ActiveSupport::Concern + GLOBAL_SCOPE = :limitable_global_scope included do class_attribute :limit_scope @@ -14,14 +15,34 @@ module Limitable private def validate_plan_limit_not_exceeded + if GLOBAL_SCOPE == limit_scope + validate_global_plan_limit_not_exceeded + else + validate_scoped_plan_limit_not_exceeded + end + end + + def validate_scoped_plan_limit_not_exceeded scope_relation = self.public_send(limit_scope) # rubocop:disable GitlabSecurity/PublicSend return unless scope_relation relation = self.class.where(limit_scope => scope_relation) + limits = scope_relation.actual_limits - if scope_relation.actual_limits.exceeded?(limit_name, relation) - errors.add(:base, _("Maximum number of %{name} (%{count}) exceeded") % - { name: limit_name.humanize(capitalize: false), count: scope_relation.actual_limits.public_send(limit_name) }) # rubocop:disable GitlabSecurity/PublicSend - end + check_plan_limit_not_exceeded(limits, relation) + end + + def validate_global_plan_limit_not_exceeded + relation = self.class.all + limits = Plan.default.actual_limits + + check_plan_limit_not_exceeded(limits, relation) + end + + def check_plan_limit_not_exceeded(limits, relation) + return unless limits.exceeded?(limit_name, relation) + + errors.add(:base, _("Maximum number of %{name} (%{count}) exceeded") % + { name: limit_name.humanize(capitalize: false), count: limits.public_send(limit_name) }) # rubocop:disable GitlabSecurity/PublicSend end end diff --git a/app/services/draft_notes/base_service.rb b/app/services/draft_notes/base_service.rb new file mode 100644 index 00000000000..89daae0e8f4 --- /dev/null +++ b/app/services/draft_notes/base_service.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module DraftNotes + class BaseService < ::BaseService + attr_accessor :merge_request, :current_user, :params + + def initialize(merge_request, current_user, params = nil) + @merge_request, @current_user, @params = merge_request, current_user, params.dup + end + + private + + def draft_notes + @draft_notes ||= merge_request.draft_notes.order_id_asc.authored_by(current_user) + end + + def project + merge_request.target_project + end + end +end diff --git a/app/services/draft_notes/create_service.rb b/app/services/draft_notes/create_service.rb new file mode 100644 index 00000000000..501778b7d5f --- /dev/null +++ b/app/services/draft_notes/create_service.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module DraftNotes + class CreateService < DraftNotes::BaseService + attr_accessor :in_draft_mode, :in_reply_to_discussion_id + + def initialize(merge_request, current_user, params = nil) + @in_reply_to_discussion_id = params.delete(:in_reply_to_discussion_id) + super + end + + def execute + if in_reply_to_discussion_id.present? + unless discussion + return base_error(_('Thread to reply to cannot be found')) + end + + params[:discussion_id] = discussion.reply_id + end + + if params[:resolve_discussion] && !can_resolve_discussion? + return base_error(_('User is not allowed to resolve thread')) + end + + draft_note = DraftNote.new(params) + draft_note.merge_request = merge_request + draft_note.author = current_user + draft_note.save + + if in_reply_to_discussion_id.blank? && draft_note.diff_file&.unfolded? + merge_request.diffs.clear_cache + end + + draft_note + end + + private + + def base_error(text) + DraftNote.new.tap do |draft| + draft.errors.add(:base, text) + end + end + + def discussion + @discussion ||= merge_request.notes.find_discussion(in_reply_to_discussion_id) + end + + def can_resolve_discussion? + note = discussion&.notes&.first + return false unless note + + current_user && Ability.allowed?(current_user, :resolve_note, note) + end + end +end diff --git a/app/services/draft_notes/destroy_service.rb b/app/services/draft_notes/destroy_service.rb new file mode 100644 index 00000000000..ddca0debb03 --- /dev/null +++ b/app/services/draft_notes/destroy_service.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module DraftNotes + class DestroyService < DraftNotes::BaseService + # If no `draft` is given it fallsback to all + # draft notes of the given merge request and user. + def execute(draft = nil) + drafts = draft || draft_notes + + clear_highlight_diffs_cache(Array.wrap(drafts)) + + drafts.is_a?(DraftNote) ? drafts.destroy! : drafts.delete_all + end + + private + + def clear_highlight_diffs_cache(drafts) + if drafts.any? { |draft| draft.diff_file&.unfolded? } + merge_request.diffs.clear_cache + end + end + end +end diff --git a/app/services/draft_notes/publish_service.rb b/app/services/draft_notes/publish_service.rb new file mode 100644 index 00000000000..a9a7304e5ed --- /dev/null +++ b/app/services/draft_notes/publish_service.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module DraftNotes + class PublishService < DraftNotes::BaseService + def execute(draft = nil) + return error('Not allowed to create notes') unless can?(current_user, :create_note, merge_request) + + if draft + publish_draft_note(draft) + else + publish_draft_notes + end + + success + rescue ActiveRecord::RecordInvalid => e + message = "Unable to save #{e.record.class.name}: #{e.record.errors.full_messages.join(", ")} " + error(message) + end + + private + + def publish_draft_note(draft) + create_note_from_draft(draft) + draft.delete + + MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(merge_request) + end + + def publish_draft_notes + return if draft_notes.empty? + + review = Review.create!(author: current_user, merge_request: merge_request, project: project) + + draft_notes.map do |draft_note| + draft_note.review = review + create_note_from_draft(draft_note) + end + draft_notes.delete_all + + notification_service.async.new_review(review) + MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(merge_request) + end + + def create_note_from_draft(draft) + # Make sure the diff file is unfolded in order to find the correct line + # codes. + draft.diff_file&.unfold_diff_lines(draft.original_position) + + note = Notes::CreateService.new(draft.project, draft.author, draft.publish_params).execute + set_discussion_resolve_status(note, draft) + + note + end + + def set_discussion_resolve_status(note, draft_note) + return unless draft_note.discussion_id.present? + + discussion = note.discussion + + if draft_note.resolve_discussion && discussion.can_resolve?(current_user) + discussion.resolve!(current_user) + else + discussion.unresolve! + end + end + end +end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 6c1f52ec866..935dbfb72dd 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -88,9 +88,11 @@ module Notes end end - # EE::Notes::CreateService would override this method def quick_action_options - { merge_request_diff_head_sha: params[:merge_request_diff_head_sha] } + { + merge_request_diff_head_sha: params[:merge_request_diff_head_sha], + review_id: params[:review_id] + } end def tracking_data_for(note) @@ -103,5 +105,3 @@ module Notes end end end - -Notes::CreateService.prepend_if_ee('EE::Notes::CreateService') diff --git a/app/services/notification_recipients/build_service.rb b/app/services/notification_recipients/build_service.rb index df807f11e1b..0fe0d26d7b2 100644 --- a/app/services/notification_recipients/build_service.rb +++ b/app/services/notification_recipients/build_service.rb @@ -32,7 +32,9 @@ module NotificationRecipients def self.build_new_release_recipients(*args) ::NotificationRecipients::Builder::NewRelease.new(*args).notification_recipients end + + def self.build_new_review_recipients(*args) + ::NotificationRecipients::Builder::NewReview.new(*args).notification_recipients + end end end - -NotificationRecipients::BuildService.prepend_if_ee('EE::NotificationRecipients::BuildService') diff --git a/app/services/notification_recipients/builder/new_review.rb b/app/services/notification_recipients/builder/new_review.rb new file mode 100644 index 00000000000..3b1296f6967 --- /dev/null +++ b/app/services/notification_recipients/builder/new_review.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module NotificationRecipients + module Builder + class NewReview < Base + attr_reader :review + def initialize(review) + @review = review + end + + def target + review.merge_request + end + + def project + review.project + end + + def group + project.group + end + + def build! + add_participants(review.author) + add_mentions(review.author, target: review) + add_project_watchers + add_custom_notifications + add_subscribed_users + end + + # A new review is a batch of new notes + # therefore new_note subscribers should also + # receive incoming new reviews + def custom_action + :new_note + end + + def acting_user + review.author + end + end + end +end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 4c1db03fab8..ae512563585 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -557,6 +557,15 @@ class NotificationService mailer.group_was_not_exported_email(current_user, group, errors).deliver_later end + # Notify users on new review in system + def new_review(review) + recipients = NotificationRecipients::BuildService.build_new_review_recipients(review) + + recipients.each do |recipient| + mailer.new_review_email(recipient.user.id, review.id).deliver_later + end + end + protected def new_resource_email(target, method) diff --git a/app/services/prometheus/proxy_variable_substitution_service.rb b/app/services/prometheus/proxy_variable_substitution_service.rb index aa3a09ba05c..7b98cfc592a 100644 --- a/app/services/prometheus/proxy_variable_substitution_service.rb +++ b/app/services/prometheus/proxy_variable_substitution_service.rb @@ -32,8 +32,8 @@ module Prometheus def validate_variables(_result) return success unless variables - unless variables.is_a?(Array) && variables.size.even? - return error(_('Optional parameter "variables" must be an array of keys and values. Ex: [key1, value1, key2, value2]')) + unless variables.is_a?(ActionController::Parameters) + return error(_('Optional parameter "variables" must be a Hash. Ex: variables[key1]=value1')) end success @@ -88,12 +88,7 @@ module Prometheus end def variables_hash - # .each_slice(2) converts ['key1', 'value1', 'key2', 'value2'] into - # [['key1', 'value1'], ['key2', 'value2']] which is then converted into - # a hash by to_h: {'key1' => 'value1', 'key2' => 'value2'} - # to_h will raise an ArgumentError if the number of elements in the original - # array is not even. - variables&.each_slice(2).to_h + variables.to_h end def query(result) diff --git a/app/services/users/migrate_to_ghost_user_service.rb b/app/services/users/migrate_to_ghost_user_service.rb index 5ca9ed67e56..1b46edd4d7d 100644 --- a/app/services/users/migrate_to_ghost_user_service.rb +++ b/app/services/users/migrate_to_ghost_user_service.rb @@ -53,6 +53,7 @@ module Users migrate_abuse_reports migrate_award_emoji migrate_snippets + migrate_reviews end # rubocop: disable CodeReuse/ActiveRecord @@ -85,6 +86,10 @@ module Users snippets = user.snippets.only_project_snippets snippets.update_all(author_id: ghost_user.id) end + + def migrate_reviews + user.reviews.update_all(author_id: ghost_user.id) + end end end diff --git a/app/views/notify/new_review_email.html.haml b/app/views/notify/new_review_email.html.haml new file mode 100644 index 00000000000..ad870473681 --- /dev/null +++ b/app/views/notify/new_review_email.html.haml @@ -0,0 +1,16 @@ +%table{ border: "0", cellpadding:"0", cellspacing: "0", style: "width:100%;margin:0 auto;border-collapse:separate;border-spacing:0;" } + %tbody + %tr + %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;background-color:#ffffff;text-align:left;overflow:hidden;" } + %table{ border: "0", cellpadding: "0", cellspacing: "0", style: "width:100%;border-collapse:separate;border-spacing:0;" } + %tbody + %tr + %td{ style: "color:#333333;border-bottom:1px solid #ededed;font-size:15px;font-weight:bold;line-height:1.4;padding: 20px 0;" } + - mr_link = link_to(@merge_request.to_reference(@project), project_merge_request_url(@project, @merge_request)) + - mr_author_link = link_to(@author_name, user_url(@author)) + = _('Merge request %{mr_link} was reviewed by %{mr_author}').html_safe % { mr_link: mr_link, mr_author: mr_author_link } + %tr + %td{ style: "overflow:hidden;font-size:14px;line-height:1.4;display:grid;" } + - @notes.each do |note| + - target_url = project_merge_request_url(@project, @merge_request, anchor: "note_#{note.id}") + = render 'note_email', note: note, diff_limit: 3, target_url: target_url, note_style: "border-bottom:1px solid #ededed;" diff --git a/app/views/notify/new_review_email.text.erb b/app/views/notify/new_review_email.text.erb new file mode 100644 index 00000000000..164735abad0 --- /dev/null +++ b/app/views/notify/new_review_email.text.erb @@ -0,0 +1,13 @@ +<% mr_url = merge_request_url(@merge_request) %> +<% mr_author_name = sanitize_name(@author_name) %> +<%= _('Merge request %{mr_link} was reviewed by %{mr_author}') % { mr_link: mr_url, mr_author: mr_author_name } %> + +-- +<% @notes.each_with_index do |note, index| %> + <% target_url = project_merge_request_url(@project, @merge_request, anchor: "note_#{note.id}") %> + <%= render 'note_email', note: note, diff_limit: 3, target_url: target_url %> + + <% if index != @notes.length-1 %> +-- + <% end %> +<% end %> diff --git a/app/views/registrations/experience_level.html.haml b/app/views/registrations/experience_level.html.haml new file mode 100644 index 00000000000..5ceed919c32 --- /dev/null +++ b/app/views/registrations/experience_level.html.haml @@ -0,0 +1,26 @@ +- content_for(:page_title, _('What’s your experience level?')) + +%h3= _('Hello there') +%p= _('Welcome to the guided GitLab tour') + +%br + +%h5= _('What describes you best?') + +%hr + +%div + %p + %b= _('Novice') + %p= _('I’m not very familiar with the basics of project management and DevOps.') + %p + %a{ href: '#novice' }= _('Show me everything') + +%hr + +%div + %p + %b= _('Experienced') + %p= _('I’m familiar with the basics of project management and DevOps.') + %p + %a{ href: '#experienced' }= _('Show me more advanced stuff') diff --git a/bin/background_jobs_sk b/bin/background_jobs_sk index fb7de0a6180..0aab69126b2 100755 --- a/bin/background_jobs_sk +++ b/bin/background_jobs_sk @@ -36,7 +36,7 @@ start_silent() start_sidekiq() { cmd="exec" - chpst=$(which chpst) + chpst=$(command -v chpst) if [ -n "$chpst" ]; then cmd="${cmd} ${chpst} -P" diff --git a/bin/background_jobs_sk_cluster b/bin/background_jobs_sk_cluster index b1d5fce204e..6188ec51420 100755 --- a/bin/background_jobs_sk_cluster +++ b/bin/background_jobs_sk_cluster @@ -43,7 +43,7 @@ restart() start_sidekiq() { cmd="exec" - chpst=$(which chpst) + chpst=$(command -v chpst) if [ -n "$chpst" ]; then cmd="${cmd} ${chpst} -P" diff --git a/changelogs/unreleased/218312-change-variables-parameter-format.yml b/changelogs/unreleased/218312-change-variables-parameter-format.yml new file mode 100644 index 00000000000..352248bbfc5 --- /dev/null +++ b/changelogs/unreleased/218312-change-variables-parameter-format.yml @@ -0,0 +1,5 @@ +--- +title: Change format of variables parameter in Prometheus proxy API for metrics dashboard +merge_request: 33062 +author: +type: fixed diff --git a/changelogs/unreleased/add-global-plans.yml b/changelogs/unreleased/add-global-plans.yml new file mode 100644 index 00000000000..cafd6cfc227 --- /dev/null +++ b/changelogs/unreleased/add-global-plans.yml @@ -0,0 +1,5 @@ +--- +title: Adapt Limitable for system-wide features +merge_request: 32574 +author: +type: added diff --git a/changelogs/unreleased/sy-publish-command.yml b/changelogs/unreleased/sy-publish-command.yml new file mode 100644 index 00000000000..cfc46e8f1c1 --- /dev/null +++ b/changelogs/unreleased/sy-publish-command.yml @@ -0,0 +1,6 @@ +--- +title: Backfill StatusPage::Published incidents and enable a publish quick action + for EE +merge_request: 30906 +author: +type: added diff --git a/config/routes.rb b/config/routes.rb index 86f42822299..dfc9be3d4f8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -46,6 +46,7 @@ Rails.application.routes.draw do # Sign up get 'users/sign_up/welcome' => 'registrations#welcome' + get 'users/sign_up/experience_level' => 'registrations#experience_level' patch 'users/sign_up/update_registration' => 'registrations#update_registration' # Search diff --git a/db/post_migrate/20200421195234_backfill_status_page_published_incidents.rb b/db/post_migrate/20200421195234_backfill_status_page_published_incidents.rb new file mode 100644 index 00000000000..fa7a5a9d924 --- /dev/null +++ b/db/post_migrate/20200421195234_backfill_status_page_published_incidents.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +class BackfillStatusPagePublishedIncidents < ActiveRecord::Migration[6.0] + DOWNTIME = false + + disable_ddl_transaction! + + class Incident < ActiveRecord::Base + self.table_name = 'status_page_published_incidents' + end + + class StatusPageIssue < ActiveRecord::Base + include ::EachBatch + + self.table_name = 'issues' + + scope :published_only, -> do + joins('INNER JOIN status_page_settings ON status_page_settings.project_id = issues.project_id') + .where('status_page_settings.enabled = true') + .where(confidential: false) + end + end + + def up + current_time = Time.current + + StatusPageIssue.published_only.each_batch do |batch| + incidents = batch.map do |status_page_issue| + { + issue_id: status_page_issue.id, + created_at: current_time, + updated_at: current_time + } + end + + Incident.insert_all(incidents, unique_by: :issue_id) + end + end + + def down + # no op + + # While we expect this table to be empty at the point of + # the up migration, there is no reliable way to determine + # whether records were added as a part of the migration + # or after it has run. + end +end diff --git a/db/structure.sql b/db/structure.sql index e30b51c576c..590760fc707 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13877,6 +13877,7 @@ COPY "schema_migrations" (version) FROM STDIN; 20200421054948 20200421092907 20200421111005 +20200421195234 20200421233150 20200422091541 20200422213749 diff --git a/doc/development/application_limits.md b/doc/development/application_limits.md index b13e2994c52..60138f23cc2 100644 --- a/doc/development/application_limits.md +++ b/doc/development/application_limits.md @@ -115,6 +115,20 @@ it_behaves_like 'includes Limitable concern' do end ``` +### Testing instance-wide limits + +Instance-wide features always use `default` Plan, as instance-wide features +do not have license assigned. + +```ruby +class InstanceVariable + include Limitable + + self.limit_name = 'instance_variables' # Optional as InstanceVariable corresponds with instance_variables + self.limit_scope = Limitable::GLOBAL_SCOPE +end +``` + ### Subscription Plans Self-managed: @@ -123,9 +137,10 @@ Self-managed: GitLab.com: -- `free` - Everyone -- `bronze`- Namespaces with a Bronze subscription -- `silver` - Namespaces with a Silver subscription -- `gold` - Namespaces with a Gold subscription +- `default` - Any system-wide feature +- `free` - Namespaces and projects with a Free subscription +- `bronze`- Namespaces and projects with a Bronze subscription +- `silver` - Namespaces and projects with a Silver subscription +- `gold` - Namespaces and projects with a Gold subscription NOTE: **Note:** The test environment doesn't have any plans. diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index 36ca6079485..95f47a809e8 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -67,6 +67,41 @@ Adhere to the [Documentation Style Guide](styleguide.md). If a style standard is See the [Structure](styleguide.md#structure) section of the [Documentation Style Guide](styleguide.md). +## Metadata + +To provide additional directives or useful information, we add metadata in YAML +format to the beginning of each product documentation page. + +For example, the following metadata would be at the beginning of a product +documentation page whose content is primarily associated with the Audit Events +feature: + +```yaml +--- +stage: Monitor +group: APM +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +--- +``` + +The following list describes the YAML parameters in use: + +- `redirect_to`: The relative path and filename (with an `.md` extension) of the + location to which visitors should be redirected for a moved page. + [Learn more](#changing-document-location). +- `stage`: The [Stage](https://about.gitlab.com/handbook/product/categories/#devops-stages) + to which the majority of the page's content belongs. +- `group`: The [Group](https://about.gitlab.com/company/team/structure/#product-groups) + to which the majority of the page's content belongs. +- `info`: The following line, which provides direction to contributors regarding + how to contact the Technical Writer associated with the page's Stage and + Group: `To determine the technical writer assigned to the Stage/Group + associated with this page, see + https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers` +- `disqus_identifier`: Identifier for Disqus commenting system. Used to keep + comments with a page that's been moved to a new URL. + [Learn more](#redirections-for-pages-with-disqus-comments). + ## Changing document location Changing a document's location requires specific steps to ensure that diff --git a/doc/install/installation.md b/doc/install/installation.md index 741c248129d..3c095004d9c 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -142,7 +142,7 @@ Starting with GitLab 12.0, Git is required to be compiled with `libpcre2`. Find out if that's the case: ```shell -ldd $(which git) | grep pcre2 +ldd $(command -v git) | grep pcre2 ``` The output should contain `libpcre2-8.so.0`. diff --git a/doc/topics/autodevops/img/guide_pipeline_stages_v12_3.png b/doc/topics/autodevops/img/guide_pipeline_stages_v12_3.png Binary files differdeleted file mode 100644 index b9bab112a9f..00000000000 --- a/doc/topics/autodevops/img/guide_pipeline_stages_v12_3.png +++ /dev/null diff --git a/doc/topics/autodevops/img/guide_pipeline_stages_v13_0.png b/doc/topics/autodevops/img/guide_pipeline_stages_v13_0.png Binary files differnew file mode 100644 index 00000000000..fb102879556 --- /dev/null +++ b/doc/topics/autodevops/img/guide_pipeline_stages_v13_0.png diff --git a/doc/topics/autodevops/index.md b/doc/topics/autodevops/index.md index 1e2264b4e48..36bbb3f77b5 100644 --- a/doc/topics/autodevops/index.md +++ b/doc/topics/autodevops/index.md @@ -393,9 +393,6 @@ To add a different cluster for each environment: 1. Navigate to your project's **{cloud-gear}** **Operations > Kubernetes**. 1. Create the Kubernetes clusters with their respective environment scope, as described from the table above. - -  - 1. After creating the clusters, navigate to each cluster and install Helm Tiller and Ingress. Wait for the Ingress IP address to be assigned. 1. Make sure you've [configured your DNS](#auto-devops-base-domain) with the @@ -408,35 +405,6 @@ and verifying your application is deployed as a Review App in the Kubernetes cluster with the `review/*` environment scope. Similarly, you can check the other environments. -## Currently supported languages - -Note that not all buildpacks support Auto Test yet, as it's a relatively new -enhancement. All of Heroku's -[officially supported languages](https://devcenter.heroku.com/articles/heroku-ci#supported-languages) -support Auto Test. The languages supported by Heroku's Herokuish buildpacks all -support Auto Test, but notably the multi-buildpack does not. - -As of GitLab 10.0, the supported buildpacks are: - -```plaintext -- heroku-buildpack-multi v1.0.0 -- heroku-buildpack-ruby v168 -- heroku-buildpack-nodejs v99 -- heroku-buildpack-clojure v77 -- heroku-buildpack-python v99 -- heroku-buildpack-java v53 -- heroku-buildpack-gradle v23 -- heroku-buildpack-scala v78 -- heroku-buildpack-play v26 -- heroku-buildpack-php v122 -- heroku-buildpack-go v72 -- heroku-buildpack-erlang fa17af9 -- buildpack-nginx v8 -``` - -If your application needs a buildpack that is not in the above list, you -might want to use a [custom buildpack](customize.md#custom-buildpacks). - ## Limitations The following restrictions apply. diff --git a/doc/topics/autodevops/quick_start_guide.md b/doc/topics/autodevops/quick_start_guide.md index 859219689f9..2662ba49e87 100644 --- a/doc/topics/autodevops/quick_start_guide.md +++ b/doc/topics/autodevops/quick_start_guide.md @@ -152,8 +152,6 @@ these steps to enable Auto DevOps if it's disabled: After you save your changes, GitLab creates a new pipeline. To view it, go to **{rocket}** **CI/CD > Pipelines**. - - In the next section, we explain what each job does in the pipeline. ## Deploy the application @@ -167,7 +165,7 @@ without refreshing the page to **{status_success}** (for success) or The jobs are separated into stages: - + - **Build** - The application builds a Docker image and uploads it to your project's [Container Registry](../../user/packages/container_registry/index.md) ([Auto Build](stages.md#auto-build)). @@ -182,8 +180,8 @@ The jobs are separated into stages: - The `dependency_scanning` job checks if the application has any dependencies susceptible to vulnerabilities and is allowed to fail ([Auto Dependency Scanning](stages.md#auto-dependency-scanning-ultimate)) **(ULTIMATE)** - - The `sast` job runs static analysis on the current code to check for potential - security issues and is allowed to fail ([Auto SAST](stages.md#auto-sast-ultimate)) **(ULTIMATE)** + - Jobs suffixed with `-sast` run static analysis on the current code to check for potential + security issues, and are allowed to fail ([Auto SAST](stages.md#auto-sast-ultimate)) **(ULTIMATE)** - The `license_management` job searches the application's dependencies to determine each of their licenses and is allowed to fail ([Auto License Compliance](stages.md#auto-license-compliance-ultimate)) **(ULTIMATE)** @@ -191,12 +189,17 @@ The jobs are separated into stages: NOTE: **Note:** All jobs except `test` are allowed to fail in the test stage. +- **Review** - Pipelines on `master` include this stage with a `dast_environment_deploy` job. + To learn more, see [Dynamic Application Security Testing (DAST)](../../user/application_security/dast/index.md). + - **Production** - After the tests and checks finish, the application deploys in Kubernetes ([Auto Deploy](stages.md#auto-deploy)). - **Performance** - Performance tests are run on the deployed application ([Auto Browser Performance Testing](stages.md#auto-browser-performance-testing-premium)). **(PREMIUM)** +- **Cleanup** - Pipelines on `master` include this stage with a `stop_dast_environment` job. + After running a pipeline, you should view your deployed website and learn how to monitor it. diff --git a/doc/topics/autodevops/stages.md b/doc/topics/autodevops/stages.md index 9017d0c6404..91b7323e3e6 100644 --- a/doc/topics/autodevops/stages.md +++ b/doc/topics/autodevops/stages.md @@ -84,11 +84,39 @@ Auto Test runs the appropriate tests for your application using your project to detect the language and framework. Several languages and frameworks are detected automatically, but if your language is not detected, you may be able to create a [custom buildpack](customize.md#custom-buildpacks). -Check the [currently supported languages](index.md#currently-supported-languages). +Check the [currently supported languages](#currently-supported-languages). Auto Test uses tests you already have in your application. If there are no tests, it's up to you to add them. +### Currently supported languages + +Note that not all buildpacks support Auto Test yet, as it's a relatively new +enhancement. All of Heroku's +[officially supported languages](https://devcenter.heroku.com/articles/heroku-ci#supported-languages) +support Auto Test. The languages supported by Heroku's Herokuish buildpacks all +support Auto Test, but notably the multi-buildpack does not. + +The supported buildpacks are: + +```plaintext +- heroku-buildpack-multi +- heroku-buildpack-ruby +- heroku-buildpack-nodejs +- heroku-buildpack-clojure +- heroku-buildpack-python +- heroku-buildpack-java +- heroku-buildpack-gradle +- heroku-buildpack-scala +- heroku-buildpack-play +- heroku-buildpack-php +- heroku-buildpack-go +- buildpack-nginx +``` + +If your application needs a buildpack that is not in the above list, you +might want to use a [custom buildpack](customize.md#custom-buildpacks). + ## Auto Code Quality **(STARTER)** Auto Code Quality uses the diff --git a/doc/topics/web_application_firewall/quick_start_guide.md b/doc/topics/web_application_firewall/quick_start_guide.md index d55ab03a3f2..79435d6a11d 100644 --- a/doc/topics/web_application_firewall/quick_start_guide.md +++ b/doc/topics/web_application_firewall/quick_start_guide.md @@ -150,7 +150,7 @@ By now you should see the pipeline running, but what is it running exactly? To navigate inside the pipeline, click its status badge (its status should be "Running"). The pipeline is split into a few stages, each running a couple of jobs. - + In the **build** stage, the application is built into a Docker image and then uploaded to your project's [Container Registry](../../user/packages/container_registry/index.md) ([Auto Build](../autodevops/stages.md#auto-build)). diff --git a/doc/user/group/epics/manage_epics.md b/doc/user/group/epics/manage_epics.md index 7da57f56101..87f20c77443 100644 --- a/doc/user/group/epics/manage_epics.md +++ b/doc/user/group/epics/manage_epics.md @@ -33,7 +33,8 @@ You will be taken to the new epic where can edit the following details: An epic's page contains the following tabs: -- **Epics and Issues**: epics and issues added to this epic. Child epics, and their issues, are shown in a tree view. +- **Epics and Issues**: epics and issues added to this epic. Child epics, and their issues, are + shown in a tree view. - Click the <kbd>></kbd> beside a parent epic to reveal the child epics and issues. - Hover over the total counts to see a breakdown of open and closed items. - **Roadmap**: a roadmap view of child epics which have start and due dates. @@ -137,8 +138,8 @@ confidential** checkbox. ### Enable Confidential Epics **(PREMIUM ONLY)** -The Confidential Epics feature is under development and not ready for production use. It's deployed behind a -feature flag that is **disabled by default**. +The Confidential Epics feature is under development and not ready for production use. +It's deployed behind a feature flag that is **disabled by default**. [GitLab administrators with access to the GitLab Rails console](../../../administration/feature_flags.md) can enable it for your instance. @@ -208,7 +209,7 @@ To remove an issue from an epic: > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/9367) in GitLab 12.5. New issues are added to the top of their list in the **Epics and Issues** tab. -You can reorder the list of issues. Issues and child epics cannot be intermingled. +You can reorder the list of issues. To reorder issues assigned to an epic: @@ -225,7 +226,7 @@ To reorder child epics assigned to an epic: > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/33039) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 13.0. New issues are added to the top of their list in the **Epics and Issues** -tab. You can move issues from one epic to another. Issues and child epics cannot be intermingled. +tab. You can move issues from one epic to another. To move an issue to another epic: @@ -235,7 +236,7 @@ To move an issue to another epic: ### Promote an issue to an epic > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/3777) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 11.6. -> - In [GitLab 12.8](https://gitlab.com/gitlab-org/gitlab/-/issues/37081), it was moved to the Premium tier. +> - [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/37081) to [GitLab Premium](https://about.gitlab.com/pricing/) in 12.8. If you have [permissions](../../permissions.md) to close an issue and create an epic in the parent group, you can promote an issue to an epic with the `/promote` @@ -266,7 +267,8 @@ To add a child epic to an epic: 1. Click **Add an epic**. 1. Identify the epic to be added, using either of the following methods: - Paste the link of the epic. - - Search for the desired issue by entering part of the epic's title, then selecting the desired match (introduced in [GitLab 12.5](https://gitlab.com/gitlab-org/gitlab/-/issues/9126)). + - Search for the desired issue by entering part of the epic's title, then selecting the desired + match (introduced in [GitLab 12.5](https://gitlab.com/gitlab-org/gitlab/-/issues/9126)). If there are multiple epics to be added, press <kbd>Spacebar</kbd> and repeat this step. 1. Click **Add**. @@ -290,7 +292,7 @@ To move child epics to another epic: > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/9367) in GitLab 12.5. New child epics are added to the top of their list in the **Epics and Issues** tab. -You can reorder the list of child epics. Issues and child epics cannot be intermingled. +You can reorder the list of child epics. To reorder child epics assigned to an epic: diff --git a/doc/user/project/settings/project_access_tokens.md b/doc/user/project/settings/project_access_tokens.md index 303a6f6d3be..460a5b6f88d 100644 --- a/doc/user/project/settings/project_access_tokens.md +++ b/doc/user/project/settings/project_access_tokens.md @@ -1,6 +1,13 @@ -# Project access tokens **(CORE ONLY)** +# Project access tokens (Alpha) **(CORE ONLY)** -> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/2587) in GitLab 13.0. +CAUTION: **Warning:** +This is an [Alpha](https://about.gitlab.com/handbook/product/#alpha) feature, and it is subject to change at any time without +prior notice. + +> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/2587) in GitLab 13.0. +> - It's deployed behind a feature flag, disabled by default. +> - It's disabled on GitLab.com. +> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-project-access-tokens). Project access tokens are scoped to a project and can be used to authenticate with the [GitLab API](../../../api/README.md#personalproject-access-tokens). @@ -53,3 +60,22 @@ the following table. | `write_registry` | Allows write-access (push) to [container registry](../../packages/container_registry/index.md). | | `read_repository` | Allows read-only access (pull) to the repository. | | `write_repository` | Allows read-write access (pull, push) to the repository. | + +### Enable or disable project access tokens + +Project access tokens is an [Alpha](https://about.gitlab.com/handbook/product/#alpha) feature and is not recommended for production use. +It is deployed behind a feature flag that is **disabled by default**. +[GitLab administrators with access to the GitLab Rails console](../../../administration/feature_flags.md) +can enable it for your instance. + +To enable it: + +```ruby +Feature.enable(:resource_access_token) +``` + +To disable it: + +```ruby +Feature.disable(:resource_access_token) +``` diff --git a/lib/object_storage/direct_upload.rb b/lib/object_storage/direct_upload.rb index fd26663fef0..b3c0e68dbb3 100644 --- a/lib/object_storage/direct_upload.rb +++ b/lib/object_storage/direct_upload.rb @@ -2,7 +2,7 @@ module ObjectStorage # - # The DirectUpload c;ass generates a set of presigned URLs + # The DirectUpload class generates a set of presigned URLs # that can be used to upload data to object storage from untrusted component: Workhorse, Runner? # # For Google it assumes that the platform supports variable Content-Length. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index cdd98ff85a8..eb14b11e80a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8995,6 +8995,9 @@ msgstr "" msgid "Expand up" msgstr "" +msgid "Experienced" +msgstr "" + msgid "Expiration" msgstr "" @@ -9241,6 +9244,9 @@ msgstr "" msgid "Failed to protect the environment" msgstr "" +msgid "Failed to publish issue on status page." +msgstr "" + msgid "Failed to remove a Zoom meeting" msgstr "" @@ -12085,6 +12091,9 @@ msgstr "" msgid "Issue or Merge Request ID is required" msgstr "" +msgid "Issue published on status page." +msgstr "" + msgid "Issue template (optional)" msgstr "" @@ -12202,6 +12211,12 @@ msgstr "" msgid "It's you" msgstr "" +msgid "Iteration changed to" +msgstr "" + +msgid "Iteration removed" +msgstr "" + msgid "Iterations" msgstr "" @@ -12214,6 +12229,12 @@ msgstr "" msgid "Iteration|cannot be more than 500 years in the future" msgstr "" +msgid "I’m familiar with the basics of project management and DevOps." +msgstr "" + +msgid "I’m not very familiar with the basics of project management and DevOps." +msgstr "" + msgid "Jaeger URL" msgstr "" @@ -13474,6 +13495,9 @@ msgstr "" msgid "Merge request %{iid} authored by %{authorName}" msgstr "" +msgid "Merge request %{mr_link} was reviewed by %{mr_author}" +msgstr "" + msgid "Merge request approvals" msgstr "" @@ -14797,6 +14821,9 @@ msgstr "" msgid "November" msgstr "" +msgid "Novice" +msgstr "" + msgid "Now you can access the merge request navigation tabs at the top, where they’re easier to find." msgstr "" @@ -15036,7 +15063,7 @@ msgstr "" msgid "Optional" msgstr "" -msgid "Optional parameter \"variables\" must be an array of keys and values. Ex: [key1, value1, key2, value2]" +msgid "Optional parameter \"variables\" must be a Hash. Ex: variables[key1]=value1" msgstr "" msgid "Optionally, you can %{link_to_customize} how FogBugz email addresses and usernames are imported into GitLab." @@ -17550,6 +17577,12 @@ msgstr "" msgid "Public projects Minutes cost factor" msgstr "" +msgid "Publish to status page" +msgstr "" + +msgid "Publishes this issue to the associated status page." +msgstr "" + msgid "Pull" msgstr "" @@ -19868,9 +19901,15 @@ msgstr "" msgid "Show latest version" msgstr "" +msgid "Show me everything" +msgstr "" + msgid "Show me how" msgstr "" +msgid "Show me more advanced stuff" +msgstr "" + msgid "Show only direct members" msgstr "" @@ -24648,12 +24687,21 @@ msgstr "" msgid "Welcome to the Guided GitLab Tour" msgstr "" +msgid "Welcome to the guided GitLab tour" +msgstr "" + msgid "Welcome to your Issue Board!" msgstr "" msgid "What are you searching for?" msgstr "" +msgid "What describes you best?" +msgstr "" + +msgid "What’s your experience level?" +msgstr "" + msgid "When a deployment job is successful, skip older deployment jobs that are still pending" msgstr "" @@ -26006,10 +26054,10 @@ msgstr "" msgid "exceeds the limit of %{bytes} bytes for directory name \"%{dirname}\"" msgstr "" -msgid "expired on %{milestone_due_date}" +msgid "expired on %{timebox_due_date}" msgstr "" -msgid "expires on %{milestone_due_date}" +msgid "expires on %{timebox_due_date}" msgstr "" msgid "external_url" @@ -26784,10 +26832,10 @@ msgstr "" msgid "started a discussion on %{design_link}" msgstr "" -msgid "started on %{milestone_start_date}" +msgid "started on %{timebox_start_date}" msgstr "" -msgid "starts on %{milestone_start_date}" +msgid "starts on %{timebox_start_date}" msgstr "" msgid "stuck" diff --git a/qa/qa/git/repository.rb b/qa/qa/git/repository.rb index bd2fbbd80cb..43608827b2e 100644 --- a/qa/qa/git/repository.rb +++ b/qa/qa/git/repository.rb @@ -111,7 +111,7 @@ module QA end def commit_with_gpg(message) - run(%Q{git config user.signingkey #{@gpg_key_id} && git config gpg.program $(which gpg) && git commit -S -m "#{message}"}).to_s + run(%Q{git config user.signingkey #{@gpg_key_id} && git config gpg.program $(command -v gpg) && git commit -S -m "#{message}"}).to_s end def push_changes(branch = 'master') diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb index 64f90e44bb6..fb8da52930c 100644 --- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb +++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb @@ -84,12 +84,12 @@ describe Projects::Environments::PrometheusApiController do before do expected_params[:query] = %{up{pod_name="#{pod_name}"}} - expected_params[:variables] = ['pod_name', pod_name] + expected_params[:variables] = { 'pod_name' => pod_name } end it 'replaces variables with values' do get :proxy, params: environment_params.merge( - query: 'up{pod_name="{{pod_name}}"}', variables: ['pod_name', pod_name] + query: 'up{pod_name="{{pod_name}}"}', variables: { 'pod_name' => pod_name } ) expect(response).to have_gitlab_http_status(:success) diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 01a9647a763..3a6ddfb1783 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -445,4 +445,27 @@ describe RegistrationsController do end end end + + describe '#experience_level' do + subject { get :experience_level } + + let_it_be(:user) { create(:user) } + + let(:part_of_onboarding_issues_experiment) { false } + + before do + stub_experiment_for_user(onboarding_issues: part_of_onboarding_issues_experiment) + sign_in(user) + end + + context 'when not part of the onboarding issues experiment' do + it { is_expected.to have_gitlab_http_status(:not_found) } + end + + context 'when part of the onboarding issues experiment' do + let(:part_of_onboarding_issues_experiment) { true } + + it { is_expected.to render_template(:experience_level) } + end + end end diff --git a/spec/frontend/ide/components/repo_commit_section_spec.js b/spec/frontend/ide/components/repo_commit_section_spec.js index 237be018807..d4e4e064a52 100644 --- a/spec/frontend/ide/components/repo_commit_section_spec.js +++ b/spec/frontend/ide/components/repo_commit_section_spec.js @@ -2,6 +2,7 @@ import { mount } from '@vue/test-utils'; import { createStore } from '~/ide/stores'; import router from '~/ide/ide_router'; import RepoCommitSection from '~/ide/components/repo_commit_section.vue'; +import EmptyState from '~/ide/components/commit_sidebar/empty_state.vue'; import { stageKeys } from '~/ide/constants'; import { file } from '../helpers'; @@ -63,7 +64,7 @@ describe('RepoCommitSection', () => { wrapper.destroy(); }); - describe('empty Stage', () => { + describe('empty state', () => { beforeEach(() => { store.state.noChangesStateSvgPath = TEST_NO_CHANGES_SVG; store.state.committedStateSvgPath = 'svg'; @@ -74,11 +75,16 @@ describe('RepoCommitSection', () => { it('renders no changes text', () => { expect( wrapper - .find('.js-empty-state') + .find(EmptyState) .text() .trim(), ).toContain('No changes'); - expect(wrapper.find('.js-empty-state img').attributes('src')).toBe(TEST_NO_CHANGES_SVG); + expect( + wrapper + .find(EmptyState) + .find('img') + .attributes('src'), + ).toBe(TEST_NO_CHANGES_SVG); }); }); @@ -109,6 +115,10 @@ describe('RepoCommitSection', () => { expect(changedFileNames).toEqual(allFiles.map(x => x.path)); }); + + it('does not show empty state', () => { + expect(wrapper.find(EmptyState).exists()).toBe(false); + }); }); describe('with unstaged file', () => { @@ -129,5 +139,9 @@ describe('RepoCommitSection', () => { keyPrefix: stageKeys.unstaged, }); }); + + it('does not show empty state', () => { + expect(wrapper.find(EmptyState).exists()).toBe(false); + }); }); }); diff --git a/spec/frontend/ide/stores/actions/file_spec.js b/spec/frontend/ide/stores/actions/file_spec.js index 43cb06f5d92..e50697af5eb 100644 --- a/spec/frontend/ide/stores/actions/file_spec.js +++ b/spec/frontend/ide/stores/actions/file_spec.js @@ -587,20 +587,6 @@ describe('IDE store file actions', () => { }) .catch(done.fail); }); - - it('bursts unused seal', done => { - store - .dispatch('changeFileContent', { - path: tmpFile.path, - content: 'content', - }) - .then(() => { - expect(store.state.unusedSeal).toBe(false); - - done(); - }) - .catch(done.fail); - }); }); describe('with changed file', () => { diff --git a/spec/frontend/ide/stores/actions_spec.js b/spec/frontend/ide/stores/actions_spec.js index d52b0435906..666ed8a24aa 100644 --- a/spec/frontend/ide/stores/actions_spec.js +++ b/spec/frontend/ide/stores/actions_spec.js @@ -292,21 +292,6 @@ describe('Multi-file store actions', () => { }) .catch(done.fail); }); - - it('bursts unused seal', done => { - store - .dispatch('createTempEntry', { - name: 'test', - branchId: 'mybranch', - type: 'blob', - }) - .then(() => { - expect(store.state.unusedSeal).toBe(false); - - done(); - }) - .catch(done.fail); - }); }); }); @@ -682,19 +667,6 @@ describe('Multi-file store actions', () => { }); }); }); - - it('bursts unused seal', done => { - store.state.entries.test = file('test'); - - store - .dispatch('deleteEntry', 'test') - .then(() => { - expect(store.state.unusedSeal).toBe(false); - - done(); - }) - .catch(done.fail); - }); }); describe('renameEntry', () => { @@ -839,20 +811,6 @@ describe('Multi-file store actions', () => { .then(done) .catch(done.fail); }); - - it('bursts unused seal', done => { - store - .dispatch('renameEntry', { - path: 'orig', - name: 'renamed', - }) - .then(() => { - expect(store.state.unusedSeal).toBe(false); - - done(); - }) - .catch(done.fail); - }); }); describe('folder', () => { diff --git a/spec/frontend/ide/stores/mutations/file_spec.js b/spec/frontend/ide/stores/mutations/file_spec.js index 9b96b910fcb..cd308ee9991 100644 --- a/spec/frontend/ide/stores/mutations/file_spec.js +++ b/spec/frontend/ide/stores/mutations/file_spec.js @@ -356,14 +356,6 @@ describe('IDE store file mutations', () => { expect(localState.changedFiles.length).toBe(1); }); - - it('bursts unused seal', () => { - expect(localState.unusedSeal).toBe(true); - - mutations.ADD_FILE_TO_CHANGED(localState, localFile.path); - - expect(localState.unusedSeal).toBe(false); - }); }); describe('REMOVE_FILE_FROM_CHANGED', () => { @@ -374,14 +366,6 @@ describe('IDE store file mutations', () => { expect(localState.changedFiles.length).toBe(0); }); - - it('bursts unused seal', () => { - expect(localState.unusedSeal).toBe(true); - - mutations.REMOVE_FILE_FROM_CHANGED(localState, localFile.path); - - expect(localState.unusedSeal).toBe(false); - }); }); describe.each` @@ -533,19 +517,6 @@ describe('IDE store file mutations', () => { }, ); - describe('STAGE_CHANGE', () => { - it('bursts unused seal', () => { - expect(localState.unusedSeal).toBe(true); - - mutations.STAGE_CHANGE(localState, { - path: localFile.path, - diffInfo: localStore.getters.getDiffInfo(localFile.path), - }); - - expect(localState.unusedSeal).toBe(false); - }); - }); - describe('TOGGLE_FILE_CHANGED', () => { it('updates file changed status', () => { mutations.TOGGLE_FILE_CHANGED(localState, { diff --git a/spec/frontend/ide/stores/mutations_spec.js b/spec/frontend/ide/stores/mutations_spec.js index 2eca9acb8d8..55cc6eb66ab 100644 --- a/spec/frontend/ide/stores/mutations_spec.js +++ b/spec/frontend/ide/stores/mutations_spec.js @@ -265,16 +265,6 @@ describe('Multi-file store mutations', () => { expect(localState.changedFiles).toEqual([]); expect(localState.stagedFiles).toEqual([]); }); - - it('bursts unused seal', () => { - localState.entries.test = file('test'); - - expect(localState.unusedSeal).toBe(true); - - mutations.DELETE_ENTRY(localState, 'test'); - - expect(localState.unusedSeal).toBe(false); - }); }); describe('UPDATE_FILE_AFTER_COMMIT', () => { diff --git a/spec/frontend/monitoring/store/getters_spec.js b/spec/frontend/monitoring/store/getters_spec.js index 9f17dda3b9f..c7d0fb119de 100644 --- a/spec/frontend/monitoring/store/getters_spec.js +++ b/spec/frontend/monitoring/store/getters_spec.js @@ -329,7 +329,7 @@ describe('Monitoring store Getters', () => { }); }); - describe('getCustomVariablesArray', () => { + describe('getCustomVariablesParams', () => { let state; beforeEach(() => { @@ -340,25 +340,21 @@ describe('Monitoring store Getters', () => { it('transforms the variables object to an array in the [variable, variable_value] format for all variable types', () => { mutations[types.SET_VARIABLES](state, mockTemplatingDataResponses.allVariableTypes); - const variablesArray = getters.getCustomVariablesArray(state); - - expect(variablesArray).toEqual([ - 'simpleText', - 'Simple text', - 'advText', - 'default', - 'simpleCustom', - 'value1', - 'advCustomNormal', - 'value2', - ]); + const variablesArray = getters.getCustomVariablesParams(state); + + expect(variablesArray).toEqual({ + 'variables[advCustomNormal]': 'value2', + 'variables[advText]': 'default', + 'variables[simpleCustom]': 'value1', + 'variables[simpleText]': 'Simple text', + }); }); it('transforms the variables object to an empty array when no keys are present', () => { mutations[types.SET_VARIABLES](state, {}); - const variablesArray = getters.getCustomVariablesArray(state); + const variablesArray = getters.getCustomVariablesParams(state); - expect(variablesArray).toEqual([]); + expect(variablesArray).toEqual({}); }); }); diff --git a/spec/helpers/milestones_helper_spec.rb b/spec/helpers/timeboxes_helper_spec.rb index 4ce7143bdf0..6fe738914ce 100644 --- a/spec/helpers/milestones_helper_spec.rb +++ b/spec/helpers/timeboxes_helper_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MilestonesHelper do +describe TimeboxesHelper do describe '#milestones_filter_dropdown_path' do let(:project) { create(:project) } let(:project2) { create(:project) } @@ -39,23 +39,34 @@ describe MilestonesHelper do end end - describe "#milestone_date_range" do - def result_for(*args) - milestone_date_range(build(:milestone, *args)) - end - + describe "#timebox_date_range" do let(:yesterday) { Date.yesterday } let(:tomorrow) { yesterday + 2 } let(:format) { '%b %-d, %Y' } let(:yesterday_formatted) { yesterday.strftime(format) } let(:tomorrow_formatted) { tomorrow.strftime(format) } - it { expect(result_for(due_date: nil, start_date: nil)).to be_nil } - it { expect(result_for(due_date: tomorrow)).to eq("expires on #{tomorrow_formatted}") } - it { expect(result_for(due_date: yesterday)).to eq("expired on #{yesterday_formatted}") } - it { expect(result_for(start_date: tomorrow)).to eq("starts on #{tomorrow_formatted}") } - it { expect(result_for(start_date: yesterday)).to eq("started on #{yesterday_formatted}") } - it { expect(result_for(start_date: yesterday, due_date: tomorrow)).to eq("#{yesterday_formatted}–#{tomorrow_formatted}") } + context 'milestone' do + def result_for(*args) + timebox_date_range(build(:milestone, *args)) + end + + it { expect(result_for(due_date: nil, start_date: nil)).to be_nil } + it { expect(result_for(due_date: tomorrow)).to eq("expires on #{tomorrow_formatted}") } + it { expect(result_for(due_date: yesterday)).to eq("expired on #{yesterday_formatted}") } + it { expect(result_for(start_date: tomorrow)).to eq("starts on #{tomorrow_formatted}") } + it { expect(result_for(start_date: yesterday)).to eq("started on #{yesterday_formatted}") } + it { expect(result_for(start_date: yesterday, due_date: tomorrow)).to eq("#{yesterday_formatted}–#{tomorrow_formatted}") } + end + + context 'iteration' do + # Iterations always have start and due dates, so only A-B format is expected + it 'formats properly' do + iteration = build(:iteration, start_date: yesterday, due_date: tomorrow) + + expect(timebox_date_range(iteration)).to eq("#{yesterday_formatted}–#{tomorrow_formatted}") + end + end end describe '#milestone_counts' do diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 1a4f1123c73..8b99cc41a53 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -1726,4 +1726,59 @@ describe Notify do is_expected.to have_body_text target_url end end + + describe 'merge request reviews' do + let!(:review) { create(:review, project: project, merge_request: merge_request) } + let!(:notes) { create_list(:note, 3, review: review, project: project, author: review.author, noteable: merge_request) } + + subject { described_class.new_review_email(recipient.id, review.id) } + + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { review.merge_request } + end + + it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like 'an unsubscribeable thread' + + it 'is sent to the given recipient as the author' do + sender = subject.header[:from].addrs[0] + + aggregate_failures do + expect(sender.display_name).to eq(review.author_name) + expect(sender.address).to eq(gitlab_sender) + expect(subject).to deliver_to(recipient.notification_email) + end + end + + it 'contains the message from the notes of the review' do + review.notes.each do |note| + is_expected.to have_body_text note.note + end + end + + context 'when diff note' do + let!(:notes) { create_list(:diff_note_on_merge_request, 3, review: review, project: project, author: review.author, noteable: merge_request) } + + it 'links to notes' do + review.notes.each do |note| + # Text part + expect(subject.text_part.body.raw_source).to include( + project_merge_request_url(project, merge_request, anchor: "note_#{note.id}") + ) + end + end + end + + it 'contains review author name' do + is_expected.to have_body_text review.author_name + end + + it 'has the correct subject and body' do + aggregate_failures do + is_expected.to have_subject "Re: #{project.name} | #{merge_request.title} (#{merge_request.to_reference})" + + is_expected.to have_body_text project_merge_request_path(project, merge_request) + end + end + end end diff --git a/spec/migrations/backfill_status_page_published_incidents_spec.rb b/spec/migrations/backfill_status_page_published_incidents_spec.rb new file mode 100644 index 00000000000..ccdc8be4168 --- /dev/null +++ b/spec/migrations/backfill_status_page_published_incidents_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20200421195234_backfill_status_page_published_incidents.rb') + +describe BackfillStatusPagePublishedIncidents, :migration do + subject(:migration) { described_class.new } + + describe '#up' do + let(:projects) { table(:projects) } + let(:status_page_settings) { table(:status_page_settings) } + let(:issues) { table(:issues) } + let(:incidents) { table(:status_page_published_incidents) } + + let(:namespace) { table(:namespaces).create!(name: 'gitlab', path: 'gitlab') } + let(:project_without_status_page) { projects.create!(namespace_id: namespace.id) } + let(:enabled_project) { projects.create!(namespace_id: namespace.id) } + let(:disabled_project) { projects.create!(namespace_id: namespace.id) } + + let!(:enabled_setting) { status_page_settings.create!(enabled: true, project_id: enabled_project.id, **status_page_setting_attrs) } + let!(:disabled_setting) { status_page_settings.create!(enabled: false, project_id: disabled_project.id, **status_page_setting_attrs) } + + let!(:published_issue) { issues.create!(confidential: false, project_id: enabled_project.id) } + let!(:nonpublished_issue_1) { issues.create!(confidential: true, project_id: enabled_project.id) } + let!(:nonpublished_issue_2) { issues.create!(confidential: false, project_id: disabled_project.id) } + let!(:nonpublished_issue_3) { issues.create!(confidential: false, project_id: project_without_status_page.id) } + + let(:current_time) { Time.current.change(usec: 0) } + let(:status_page_setting_attrs) do + { + aws_s3_bucket_name: 'bucket', + aws_region: 'region', + aws_access_key: 'key', + encrypted_aws_secret_key: 'abc123', + encrypted_aws_secret_key_iv: 'abc123' + } + end + + it 'creates a StatusPage::PublishedIncident record for each published issue' do + Timecop.freeze(current_time) do + expect(incidents.all).to be_empty + + migrate! + + incident = incidents.first + + expect(incidents.count).to eq(1) + expect(incident.issue_id).to eq(published_issue.id) + expect(incident.created_at).to eq(current_time) + expect(incident.updated_at).to eq(current_time) + end + end + end +end diff --git a/spec/models/concerns/limitable_spec.rb b/spec/models/concerns/limitable_spec.rb new file mode 100644 index 00000000000..ca0a257be7a --- /dev/null +++ b/spec/models/concerns/limitable_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Limitable do + let(:minimal_test_class) do + Class.new do + include ActiveModel::Model + + def self.name + 'TestClass' + end + + include Limitable + end + end + + before do + stub_const("MinimalTestClass", minimal_test_class) + end + + it { expect(MinimalTestClass.limit_name).to eq('test_classes') } + + context 'with scoped limit' do + before do + MinimalTestClass.limit_scope = :project + end + + it { expect(MinimalTestClass.limit_scope).to eq(:project) } + + it 'triggers scoped validations' do + instance = MinimalTestClass.new + + expect(instance).to receive(:validate_scoped_plan_limit_not_exceeded) + + instance.valid?(:create) + end + end + + context 'with global limit' do + before do + MinimalTestClass.limit_scope = Limitable::GLOBAL_SCOPE + end + + it { expect(MinimalTestClass.limit_scope).to eq(Limitable::GLOBAL_SCOPE) } + + it 'triggers scoped validations' do + instance = MinimalTestClass.new + + expect(instance).to receive(:validate_global_plan_limit_not_exceeded) + + instance.valid?(:create) + end + end +end diff --git a/spec/services/draft_notes/create_service_spec.rb b/spec/services/draft_notes/create_service_spec.rb new file mode 100644 index 00000000000..8f244ed386b --- /dev/null +++ b/spec/services/draft_notes/create_service_spec.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe DraftNotes::CreateService do + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.target_project } + let(:user) { merge_request.author } + + def create_draft(params) + described_class.new(merge_request, user, params).execute + end + + it 'creates a simple draft note' do + draft = create_draft(note: 'This is a test') + + expect(draft).to be_an_instance_of(DraftNote) + expect(draft.note).to eq('This is a test') + expect(draft.author).to eq(user) + expect(draft.project).to eq(merge_request.target_project) + expect(draft.discussion_id).to be_nil + end + + it 'cannot resolve when there is nothing to resolve' do + draft = create_draft(note: 'Not a reply!', resolve_discussion: true) + + expect(draft.errors[:base]).to include('User is not allowed to resolve thread') + expect(draft).not_to be_persisted + end + + context 'in a thread' do + it 'creates a draft note with discussion_id' do + discussion = create(:discussion_note_on_merge_request, noteable: merge_request, project: project).discussion + + draft = create_draft(note: 'A reply!', in_reply_to_discussion_id: discussion.reply_id) + + expect(draft.note).to eq('A reply!') + expect(draft.discussion_id).to eq(discussion.reply_id) + expect(draft.resolve_discussion).to be_falsey + end + + it 'creates a draft that resolves the thread' do + discussion = create(:discussion_note_on_merge_request, noteable: merge_request, project: project).discussion + + draft = create_draft(note: 'A reply!', in_reply_to_discussion_id: discussion.reply_id, resolve_discussion: true) + + expect(draft.note).to eq('A reply!') + expect(draft.discussion_id).to eq(discussion.reply_id) + expect(draft.resolve_discussion).to be true + end + end + + it 'creates a draft note with a position in a diff' do + diff_refs = project.commit(RepoHelpers.sample_commit.id).try(:diff_refs) + + position = Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: diff_refs + ) + + draft = create_draft(note: 'Comment on diff', position: position.to_json) + + expect(draft.note).to eq('Comment on diff') + expect(draft.original_position.to_json).to eq(position.to_json) + end + + context 'diff highlight cache clearing' do + context 'when diff file is unfolded and it is not a reply' do + it 'clears diff highlighting cache' do + expect_next_instance_of(DraftNote) do |draft| + allow(draft).to receive_message_chain(:diff_file, :unfolded?) { true } + end + + expect(merge_request).to receive_message_chain(:diffs, :clear_cache) + + create_draft(note: 'This is a test') + end + end + + context 'when diff file is not unfolded and it is not a reply' do + it 'clears diff highlighting cache' do + expect_next_instance_of(DraftNote) do |draft| + allow(draft).to receive_message_chain(:diff_file, :unfolded?) { false } + end + + expect(merge_request).not_to receive(:diffs) + + create_draft(note: 'This is a test') + end + end + end +end diff --git a/spec/services/draft_notes/destroy_service_spec.rb b/spec/services/draft_notes/destroy_service_spec.rb new file mode 100644 index 00000000000..d0bf88dcdbe --- /dev/null +++ b/spec/services/draft_notes/destroy_service_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe DraftNotes::DestroyService do + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.target_project } + let(:user) { merge_request.author } + + def destroy(draft_note = nil) + DraftNotes::DestroyService.new(merge_request, user).execute(draft_note) + end + + it 'destroys a single draft note' do + drafts = create_list(:draft_note, 2, merge_request: merge_request, author: user) + + expect { destroy(drafts.first) } + .to change { DraftNote.count }.by(-1) + + expect(DraftNote.count).to eq(1) + end + + it 'destroys all draft notes for a user in a merge request' do + create_list(:draft_note, 2, merge_request: merge_request, author: user) + + expect { destroy }.to change { DraftNote.count }.by(-2) + expect(DraftNote.count).to eq(0) + end + + context 'diff highlight cache clearing' do + context 'when destroying all draft notes of a user' do + it 'clears highlighting cache if unfold required for any' do + drafts = create_list(:draft_note, 2, merge_request: merge_request, author: user) + + allow_any_instance_of(DraftNote).to receive_message_chain(:diff_file, :unfolded?) { true } + expect(merge_request).to receive_message_chain(:diffs, :clear_cache) + + destroy(drafts.first) + end + end + + context 'when destroying one draft note' do + it 'clears highlighting cache if unfold required' do + create_list(:draft_note, 2, merge_request: merge_request, author: user) + + allow_any_instance_of(DraftNote).to receive_message_chain(:diff_file, :unfolded?) { true } + expect(merge_request).to receive_message_chain(:diffs, :clear_cache) + + destroy + end + end + end +end diff --git a/spec/services/draft_notes/publish_service_spec.rb b/spec/services/draft_notes/publish_service_spec.rb new file mode 100644 index 00000000000..4ebae2f9aa2 --- /dev/null +++ b/spec/services/draft_notes/publish_service_spec.rb @@ -0,0 +1,261 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe DraftNotes::PublishService do + include RepoHelpers + + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.target_project } + let(:user) { merge_request.author } + let(:commit) { project.commit(sample_commit.id) } + + let(:position) do + Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: commit.diff_refs + ) + end + + def publish(draft: nil) + DraftNotes::PublishService.new(merge_request, user).execute(draft) + end + + context 'single draft note' do + let(:commit_id) { nil } + let!(:drafts) { create_list(:draft_note, 2, merge_request: merge_request, author: user, commit_id: commit_id, position: position) } + + it 'publishes' do + expect { publish(draft: drafts.first) }.to change { DraftNote.count }.by(-1).and change { Note.count }.by(1) + expect(DraftNote.count).to eq(1) + end + + it 'does not skip notification', :sidekiq_might_not_need_inline do + expect(Notes::CreateService).to receive(:new).with(project, user, drafts.first.publish_params).and_call_original + expect_next_instance_of(NotificationService) do |notification_service| + expect(notification_service).to receive(:new_note) + end + + result = publish(draft: drafts.first) + + expect(result[:status]).to eq(:success) + end + + context 'commit_id is set' do + let(:commit_id) { commit.id } + + it 'creates note from draft with commit_id' do + result = publish(draft: drafts.first) + + expect(result[:status]).to eq(:success) + expect(merge_request.notes.first.commit_id).to eq(commit_id) + end + end + end + + context 'multiple draft notes' do + let(:commit_id) { nil } + + before do + create(:draft_note, merge_request: merge_request, author: user, note: 'first note', commit_id: commit_id, position: position) + create(:draft_note, merge_request: merge_request, author: user, note: 'second note', commit_id: commit_id, position: position) + end + + context 'when review fails to create' do + before do + expect_next_instance_of(Review) do |review| + allow(review).to receive(:save!).and_raise(ActiveRecord::RecordInvalid.new(review)) + end + end + + it 'does not publish any draft note' do + expect { publish }.not_to change { DraftNote.count } + end + + it 'returns an error' do + result = publish + + expect(result[:status]).to eq(:error) + expect(result[:message]).to match(/Unable to save Review/) + end + end + + it 'returns success' do + result = publish + + expect(result[:status]).to eq(:success) + end + + it 'publishes all draft notes for a user in a merge request' do + expect { publish }.to change { DraftNote.count }.by(-2).and change { Note.count }.by(2).and change { Review.count }.by(1) + expect(DraftNote.count).to eq(0) + + notes = merge_request.notes.order(id: :asc) + expect(notes.first.note).to eq('first note') + expect(notes.last.note).to eq('second note') + end + + it 'sends batch notification' do + expect_next_instance_of(NotificationService) do |notification_service| + expect(notification_service).to receive_message_chain(:async, :new_review).with(kind_of(Review)) + end + + publish + end + + context 'commit_id is set' do + let(:commit_id) { commit.id } + + it 'creates note from draft with commit_id' do + result = publish + + expect(result[:status]).to eq(:success) + + merge_request.notes.each do |note| + expect(note.commit_id).to eq(commit_id) + end + end + end + end + + context 'draft notes with suggestions' do + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + + let(:suggestion_note) do + <<-MARKDOWN.strip_heredoc + ```suggestion + foo + ``` + MARKDOWN + end + + let!(:draft) { create(:draft_note_on_text_diff, note: suggestion_note, merge_request: merge_request, author: user) } + + it 'creates a suggestion with correct content' do + expect { publish(draft: draft) }.to change { Suggestion.count }.by(1) + .and change { DiffNote.count }.from(0).to(1) + + suggestion = Suggestion.last + + expect(suggestion.from_line).to eq(14) + expect(suggestion.to_line).to eq(14) + expect(suggestion.from_content).to eq(" vars = {\n") + expect(suggestion.to_content).to eq(" foo\n") + end + + context 'when the diff is changed' do + let(:file_path) { 'files/ruby/popen.rb' } + let(:branch_name) { project.default_branch } + let(:commit) { project.repository.commit } + + def update_file(file_path, new_content) + params = { + file_path: file_path, + commit_message: "Update File", + file_content: new_content, + start_project: project, + start_branch: project.default_branch, + branch_name: branch_name + } + + Files::UpdateService.new(project, user, params).execute + end + + before do + project.add_developer(user) + end + + it 'creates a suggestion based on the latest diff content and positions' do + diff_file = merge_request.diffs(paths: [file_path]).diff_files.first + raw_data = diff_file.new_blob.data + + # Add a line break to the beginning of the file + result = update_file(file_path, raw_data.prepend("\n")) + oldrev = merge_request.diff_head_sha + newrev = result[:result] + + expect(newrev).to be_present + + # Generates new MR revision at DB level + refresh = MergeRequests::RefreshService.new(project, user) + refresh.execute(oldrev, newrev, merge_request.source_branch_ref) + + expect { publish(draft: draft) }.to change { Suggestion.count }.by(1) + .and change { DiffNote.count }.from(0).to(1) + + suggestion = Suggestion.last + + expect(suggestion.from_line).to eq(15) + expect(suggestion.to_line).to eq(15) + expect(suggestion.from_content).to eq(" vars = {\n") + expect(suggestion.to_content).to eq(" foo\n") + end + end + end + + it 'only publishes the draft notes belonging to the current user' do + other_user = create(:user) + project.add_maintainer(other_user) + + create_list(:draft_note, 2, merge_request: merge_request, author: user) + create_list(:draft_note, 2, merge_request: merge_request, author: other_user) + + expect { publish }.to change { DraftNote.count }.by(-2).and change { Note.count }.by(2) + expect(DraftNote.count).to eq(2) + end + + context 'with quick actions' do + it 'performs quick actions' do + other_user = create(:user) + project.add_developer(other_user) + + create(:draft_note, merge_request: merge_request, + author: user, + note: "thanks\n/assign #{other_user.to_reference}") + + expect { publish }.to change { DraftNote.count }.by(-1).and change { Note.count }.by(2) + expect(merge_request.reload.assignees).to match_array([other_user]) + expect(merge_request.notes.last).to be_system + end + + it 'does not create a note if it only contains quick actions' do + create(:draft_note, merge_request: merge_request, author: user, note: "/assign #{user.to_reference}") + + expect { publish }.to change { DraftNote.count }.by(-1).and change { Note.count }.by(1) + expect(merge_request.reload.assignees).to eq([user]) + expect(merge_request.notes.last).to be_system + end + end + + context 'with drafts that resolve threads' do + let!(:note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + let!(:draft_note) { create(:draft_note, merge_request: merge_request, author: user, resolve_discussion: true, discussion_id: note.discussion.reply_id) } + + it 'resolves the thread' do + publish(draft: draft_note) + + expect(note.discussion.resolved?).to be true + end + + it 'sends notifications if all threads are resolved' do + expect_next_instance_of(MergeRequests::ResolvedDiscussionNotificationService) do |instance| + expect(instance).to receive(:execute).with(merge_request) + end + + publish + end + end + + context 'user cannot create notes' do + before do + allow(Ability).to receive(:allowed?).with(user, :create_note, merge_request).and_return(false) + end + + it 'returns an error' do + expect(publish[:status]).to eq(:error) + end + end +end diff --git a/spec/services/notification_recipients/build_service_spec.rb b/spec/services/notification_recipients/build_service_spec.rb index 2e848c2f04d..e203093623d 100644 --- a/spec/services/notification_recipients/build_service_spec.rb +++ b/spec/services/notification_recipients/build_service_spec.rb @@ -58,4 +58,56 @@ describe NotificationRecipients::BuildService do end end end + + describe '#build_new_review_recipients' do + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:review) { create(:review, merge_request: merge_request, project: project, author: merge_request.author) } + let(:notes) { create_list(:note_on_merge_request, 3, review: review, noteable: review.merge_request, project: review.project) } + + shared_examples 'no N+1 queries' do + it 'avoids N+1 queries', :request_store do + create_user + + service.build_new_review_recipients(review) + + control_count = ActiveRecord::QueryRecorder.new do + service.build_new_review_recipients(review) + end + + create_user + + expect { service.build_new_review_recipients(review) }.not_to exceed_query_limit(control_count) + end + end + + context 'when there are multiple watchers' do + def create_user + watcher = create(:user) + create(:notification_setting, source: project, user: watcher, level: :watch) + + other_projects.each do |other_project| + create(:notification_setting, source: other_project, user: watcher, level: :watch) + end + end + + include_examples 'no N+1 queries' + end + + context 'when there are multiple subscribers' do + def create_user + subscriber = create(:user) + merge_request.subscriptions.create(user: subscriber, project: project, subscribed: true) + end + + include_examples 'no N+1 queries' + + context 'when the project is private' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end + + include_examples 'no N+1 queries' + end + end + end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 2a7166e3895..d3376ef0a04 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -2863,6 +2863,57 @@ describe NotificationService, :mailer do end end + describe '#new_review' do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:reviewer) { create(:user) } + let(:merge_request) { create(:merge_request, source_project: project, assignees: [user, user2], author: create(:user)) } + let(:review) { create(:review, merge_request: merge_request, project: project, author: reviewer) } + let(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, author: reviewer, review: review) } + + before do + build_team(review.project) + add_users(review.project) + add_user_subscriptions(merge_request) + project.add_maintainer(merge_request.author) + project.add_maintainer(reviewer) + merge_request.assignees.each { |assignee| project.add_maintainer(assignee) } + + create(:diff_note_on_merge_request, + project: project, + noteable: merge_request, + author: reviewer, + review: review, + note: "cc @mention") + end + + it 'sends emails' do + expect(Notify).not_to receive(:new_review_email).with(review.author.id, review.id) + expect(Notify).not_to receive(:new_review_email).with(@unsubscriber.id, review.id) + merge_request.assignee_ids.each do |assignee_id| + expect(Notify).to receive(:new_review_email).with(assignee_id, review.id).and_call_original + end + expect(Notify).to receive(:new_review_email).with(merge_request.author.id, review.id).and_call_original + expect(Notify).to receive(:new_review_email).with(@u_watcher.id, review.id).and_call_original + expect(Notify).to receive(:new_review_email).with(@u_mentioned.id, review.id).and_call_original + expect(Notify).to receive(:new_review_email).with(@subscriber.id, review.id).and_call_original + expect(Notify).to receive(:new_review_email).with(@watcher_and_subscriber.id, review.id).and_call_original + expect(Notify).to receive(:new_review_email).with(@subscribed_participant.id, review.id).and_call_original + + subject.new_review(review) + end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { review } + let(:notification_trigger) { subject.new_review(review) } + + around do |example| + perform_enqueued_jobs { example.run } + end + end + end + def build_team(project) @u_watcher = create_global_setting_for(create(:user), :watch) @u_participating = create_global_setting_for(create(:user), :participating) diff --git a/spec/services/prometheus/proxy_variable_substitution_service_spec.rb b/spec/services/prometheus/proxy_variable_substitution_service_spec.rb index 82ea356d599..5982dcbc404 100644 --- a/spec/services/prometheus/proxy_variable_substitution_service_spec.rb +++ b/spec/services/prometheus/proxy_variable_substitution_service_spec.rb @@ -64,7 +64,7 @@ describe Prometheus::ProxyVariableSubstitutionService do let(:params_keys) do { query: 'up{pod_name="{{pod_name}}"}', - variables: ['pod_name', pod_name] + variables: { 'pod_name' => pod_name } } end @@ -76,7 +76,7 @@ describe Prometheus::ProxyVariableSubstitutionService do let(:params_keys) do { query: 'up{pod_name="{{pod_name}}",env="{{ci_environment_slug}}"}', - variables: ['pod_name', pod_name, 'ci_environment_slug', 'custom_value'] + variables: { 'pod_name' => pod_name, 'ci_environment_slug' => 'custom_value' } } end @@ -95,8 +95,7 @@ describe Prometheus::ProxyVariableSubstitutionService do } end - it_behaves_like 'error', 'Optional parameter "variables" must be an ' \ - 'array of keys and values. Ex: [key1, value1, key2, value2]' + it_behaves_like 'error', 'Optional parameter "variables" must be a Hash. Ex: variables[key1]=value1' end context 'with nil variables' do diff --git a/spec/services/users/migrate_to_ghost_user_service_spec.rb b/spec/services/users/migrate_to_ghost_user_service_spec.rb index a7d7c16a66f..c2a793b2368 100644 --- a/spec/services/users/migrate_to_ghost_user_service_spec.rb +++ b/spec/services/users/migrate_to_ghost_user_service_spec.rb @@ -84,6 +84,15 @@ describe Users::MigrateToGhostUserService do end end + context 'reviews' do + let!(:user) { create(:user) } + let(:service) { described_class.new(user) } + + include_examples "migrating a deleted user's associated records to the ghost user", Review, [:author] do + let(:created_record) { create(:review, author: user) } + end + end + context "when record migration fails with a rollback exception" do before do expect_any_instance_of(ActiveRecord::Associations::CollectionProxy) diff --git a/spec/support/shared_examples/models/concerns/limitable_shared_examples.rb b/spec/support/shared_examples/models/concerns/limitable_shared_examples.rb index 4bcea36fd42..d21823661f8 100644 --- a/spec/support/shared_examples/models/concerns/limitable_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/limitable_shared_examples.rb @@ -26,7 +26,7 @@ RSpec.shared_examples 'includes Limitable concern' do subject.dup.save end - it 'cannot create new models exceding the plan limits' do + it 'cannot create new models exceeding the plan limits' do expect { subject.save }.not_to change { described_class.count } expect(subject.errors[:base]).to contain_exactly("Maximum number of #{subject.class.limit_name.humanize(capitalize: false)} (1) exceeded") end |
