diff options
94 files changed, 1481 insertions, 117 deletions
diff --git a/app/assets/javascripts/notes/components/discussion_actions.vue b/app/assets/javascripts/notes/components/discussion_actions.vue index 22cca756ef6..1357a5268d6 100644 --- a/app/assets/javascripts/notes/components/discussion_actions.vue +++ b/app/assets/javascripts/notes/components/discussion_actions.vue @@ -39,20 +39,27 @@ export default { </script> <template> - <div class="discussion-with-resolve-btn"> + <div class="discussion-with-resolve-btn clearfix"> <reply-placeholder class="qa-discussion-reply" @onClick="$emit('showReplyForm')" /> - <resolve-discussion-button - v-if="discussion.resolvable" - :is-resolving="isResolving" - :button-title="resolveButtonTitle" - @onClick="$emit('resolve')" - /> - <div v-if="discussion.resolvable" class="btn-group discussion-actions ml-sm-2" role="group"> - <resolve-with-issue-button v-if="resolveWithIssuePath" :url="resolveWithIssuePath" /> - <jump-to-next-discussion-button - v-if="shouldShowJumpToNextDiscussion" - @onClick="$emit('jumpToNextDiscussion')" + + <div class="btn-group discussion-actions" role="group"> + <resolve-discussion-button + v-if="discussion.resolvable" + :is-resolving="isResolving" + :button-title="resolveButtonTitle" + @onClick="$emit('resolve')" + /> + <resolve-with-issue-button + v-if="discussion.resolvable && resolveWithIssuePath" + :url="resolveWithIssuePath" /> </div> + + <div + v-if="discussion.resolvable && shouldShowJumpToNextDiscussion" + class="btn-group discussion-actions ml-sm-2" + > + <jump-to-next-discussion-button @onClick="$emit('jumpToNextDiscussion')" /> + </div> </div> </template> diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index 10b15a9c38c..b8eaff32cce 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -126,10 +126,7 @@ export default { return this.discussion.resolved_by_push ? __('Automatically resolved') : __('Resolved'); }, shouldShowJumpToNextDiscussion() { - return this.showJumpToNextDiscussion( - this.discussion.id, - this.discussionsByDiffOrder ? 'diff' : 'discussion', - ); + return this.showJumpToNextDiscussion(this.discussionsByDiffOrder ? 'diff' : 'discussion'); }, shouldRenderDiffs() { return this.discussion.diff_discussion && this.renderDiffFile; diff --git a/app/assets/javascripts/notes/stores/getters.js b/app/assets/javascripts/notes/stores/getters.js index d7982be3e4b..8aa8f5037b3 100644 --- a/app/assets/javascripts/notes/stores/getters.js +++ b/app/assets/javascripts/notes/stores/getters.js @@ -61,15 +61,13 @@ export const unresolvedDiscussionsCount = state => state.unresolvedDiscussionsCo export const resolvableDiscussionsCount = state => state.resolvableDiscussionsCount; export const hasUnresolvedDiscussions = state => state.hasUnresolvedDiscussions; -export const showJumpToNextDiscussion = (state, getters) => (discussionId, mode = 'discussion') => { +export const showJumpToNextDiscussion = (state, getters) => (mode = 'discussion') => { const orderedDiffs = mode !== 'discussion' ? getters.unresolvedDiscussionsIdsByDiff : getters.unresolvedDiscussionsIdsByDate; - const indexOf = orderedDiffs.indexOf(discussionId); - - return indexOf !== -1 && indexOf < orderedDiffs.length - 1; + return orderedDiffs.length > 1; }; export const isDiscussionResolved = (state, getters) => discussionId => diff --git a/app/assets/javascripts/performance_bar/components/detailed_metric.vue b/app/assets/javascripts/performance_bar/components/detailed_metric.vue index 8f3ba9779fb..d5f1cea8356 100644 --- a/app/assets/javascripts/performance_bar/components/detailed_metric.vue +++ b/app/assets/javascripts/performance_bar/components/detailed_metric.vue @@ -92,7 +92,9 @@ export default { </template> <template v-else> <tr> - <td>No {{ header.toLowerCase() }} for this request.</td> + <td> + {{ sprintf(__('No %{header} for this request.'), { header: header.toLowerCase() }) }} + </td> </tr> </template> </table> diff --git a/app/assets/javascripts/performance_bar/components/performance_bar_app.vue b/app/assets/javascripts/performance_bar/components/performance_bar_app.vue index 48515cf785c..185003c306e 100644 --- a/app/assets/javascripts/performance_bar/components/performance_bar_app.vue +++ b/app/assets/javascripts/performance_bar/components/performance_bar_app.vue @@ -5,6 +5,7 @@ import { glEmojiTag } from '~/emoji'; import detailedMetric from './detailed_metric.vue'; import requestSelector from './request_selector.vue'; import simpleMetric from './simple_metric.vue'; +import { s__ } from '~/locale'; export default { components: { @@ -35,10 +36,10 @@ export default { }, }, detailedMetrics: [ - { metric: 'pg', header: 'SQL queries', details: 'queries', keys: ['sql'] }, + { metric: 'pg', header: s__('PerformanceBar|SQL queries'), details: 'queries', keys: ['sql'] }, { metric: 'gitaly', - header: 'Gitaly calls', + header: s__('PerformanceBar|Gitaly calls'), details: 'details', keys: ['feature', 'request'], }, @@ -99,7 +100,8 @@ export default { class="current-host" :class="{ canary: currentRequest.details.host.canary }" > - <span v-html="birdEmoji"></span> {{ currentRequest.details.host.hostname }} + <span v-html="birdEmoji"></span> + {{ currentRequest.details.host.hostname }} </span> </div> <detailed-metric @@ -118,9 +120,9 @@ export default { data-toggle="modal" data-target="#modal-peek-line-profile" > - profile + {{ s__('PerformanceBar|profile') }} </button> - <a v-else :href="profileUrl"> profile </a> + <a v-else :href="profileUrl">{{ s__('PerformanceBar|profile') }}</a> </div> <simple-metric v-for="metric in $options.simpleMetrics" @@ -139,7 +141,7 @@ export default { id="peek-view-trace" class="view" > - <a :href="currentRequest.details.tracing.tracing_url"> trace </a> + <a :href="currentRequest.details.tracing.tracing_url">{{ s__('PerformanceBar|trace') }}</a> </div> <request-selector v-if="currentRequest" diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue index 41386178a1e..a79da476890 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue @@ -162,7 +162,8 @@ export default { removeWIPPath: store.removeWIPPath, sourceBranchPath: store.sourceBranchPath, ciEnvironmentsStatusPath: store.ciEnvironmentsStatusPath, - statusPath: store.statusPath, + mergeRequestBasicPath: store.mergeRequestBasicPath, + mergeRequestWidgetPath: store.mergeRequestWidgetPath, mergeActionsContentPath: store.mergeActionsContentPath, rebasePath: store.rebasePath, }; diff --git a/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js b/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js index 0bb70bfd658..1dae53039d5 100644 --- a/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js +++ b/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js @@ -30,11 +30,11 @@ export default class MRWidgetService { } poll() { - return axios.get(`${this.endpoints.statusPath}?serializer=basic`); + return axios.get(this.endpoints.mergeRequestBasicPath); } checkStatus() { - return axios.get(`${this.endpoints.statusPath}?serializer=widget`); + return axios.get(this.endpoints.mergeRequestWidgetPath); } fetchMergeActionsContent() { diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index bfa3e7f4a59..581fee7477f 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -86,7 +86,8 @@ export default class MergeRequestStore { this.mergePath = data.merge_path; this.ffOnlyEnabled = data.ff_only_enabled; this.shouldBeRebased = Boolean(data.should_be_rebased); - this.statusPath = data.status_path; + this.mergeRequestBasicPath = data.merge_request_basic_path; + this.mergeRequestWidgetPath = data.merge_request_widget_path; this.emailPatchesPath = data.email_patches_path; this.plainDiffPath = data.plain_diff_path; this.newBlobPath = data.new_blob_path; diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 824edb2869f..e880b941d67 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -657,6 +657,10 @@ $note-form-margin-left: 72px; margin-left: -1px; } + .btn-group > .discussion-create-issue-btn { + margin-left: -2px; + } + svg { height: 15px; } diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb index f2a6268b3e9..dcc272aecff 100644 --- a/app/controllers/projects/merge_requests/application_controller.rb +++ b/app/controllers/projects/merge_requests/application_controller.rb @@ -51,4 +51,11 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont Ci::Pipeline.none end end + + def close_merge_request_if_no_source_project + return if @merge_request.source_project + return unless @merge_request.open? + + @merge_request.close + end end diff --git a/app/controllers/projects/merge_requests/content_controller.rb b/app/controllers/projects/merge_requests/content_controller.rb new file mode 100644 index 00000000000..6e026b83ee3 --- /dev/null +++ b/app/controllers/projects/merge_requests/content_controller.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class Projects::MergeRequests::ContentController < Projects::MergeRequests::ApplicationController + # @merge_request.check_mergeability is not executed here since + # widget serializer calls it via mergeable? method + # but we might want to call @merge_request.check_mergeability + # for other types of serialization + + before_action :close_merge_request_if_no_source_project + around_action :allow_gitaly_ref_name_caching + + def widget + respond_to do |format| + format.json do + Gitlab::PollingInterval.set_header(response, interval: 10_000) + + serializer = MergeRequestSerializer.new(current_user: current_user, project: merge_request.project) + render json: serializer.represent(merge_request, serializer: 'widget') + end + end + end +end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index fc37ce1dbc4..7ee8e0ea8f8 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -235,12 +235,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo params[:auto_merge_strategy].present? || params[:merge_when_pipeline_succeeds].present? end - def close_merge_request_if_no_source_project - if !@merge_request.source_project && @merge_request.open? - @merge_request.close - end - end - private def ci_environments_status_on_merge_result? diff --git a/app/graphql/mutations/.keep b/app/graphql/mutations/.keep deleted file mode 100644 index e69de29bb2d..00000000000 --- a/app/graphql/mutations/.keep +++ /dev/null diff --git a/app/graphql/mutations/award_emojis/add.rb b/app/graphql/mutations/award_emojis/add.rb new file mode 100644 index 00000000000..8e050dd6d29 --- /dev/null +++ b/app/graphql/mutations/award_emojis/add.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Mutations + module AwardEmojis + class Add < Base + graphql_name 'AddAwardEmoji' + + def resolve(args) + awardable = authorized_find!(id: args[:awardable_id]) + + check_object_is_awardable!(awardable) + + # TODO this will be handled by AwardEmoji::AddService + # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63372 and + # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29782 + award = awardable.create_award_emoji(args[:name], current_user) + + { + award_emoji: (award if award.persisted?), + errors: errors_on_object(award) + } + end + end + end +end diff --git a/app/graphql/mutations/award_emojis/base.rb b/app/graphql/mutations/award_emojis/base.rb new file mode 100644 index 00000000000..d868db84f9d --- /dev/null +++ b/app/graphql/mutations/award_emojis/base.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Mutations + module AwardEmojis + class Base < BaseMutation + include Gitlab::Graphql::Authorize::AuthorizeResource + + authorize :award_emoji + + argument :awardable_id, + GraphQL::ID_TYPE, + required: true, + description: 'The global id of the awardable resource' + + argument :name, + GraphQL::STRING_TYPE, + required: true, + description: copy_field_description(Types::AwardEmojis::AwardEmojiType, :name) + + field :award_emoji, + Types::AwardEmojis::AwardEmojiType, + null: true, + description: 'The award emoji after mutation' + + private + + def find_object(id:) + GitlabSchema.object_from_id(id) + end + + # Called by mutations methods after performing an authorization check + # of an awardable object. + def check_object_is_awardable!(object) + unless object.is_a?(Awardable) && object.emoji_awardable? + raise Gitlab::Graphql::Errors::ResourceNotAvailable, + 'Cannot award emoji to this resource' + end + end + end + end +end diff --git a/app/graphql/mutations/award_emojis/remove.rb b/app/graphql/mutations/award_emojis/remove.rb new file mode 100644 index 00000000000..3ba85e445b8 --- /dev/null +++ b/app/graphql/mutations/award_emojis/remove.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Mutations + module AwardEmojis + class Remove < Base + graphql_name 'RemoveAwardEmoji' + + def resolve(args) + awardable = authorized_find!(id: args[:awardable_id]) + + check_object_is_awardable!(awardable) + + # TODO this check can be removed once AwardEmoji services are available. + # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63372 and + # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29782 + unless awardable.awarded_emoji?(args[:name], current_user) + raise Gitlab::Graphql::Errors::ResourceNotAvailable, + 'You have not awarded emoji of type name to the awardable' + end + + # TODO this will be handled by AwardEmoji::DestroyService + # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63372 and + # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29782 + awardable.remove_award_emoji(args[:name], current_user) + + { + # Mutation response is always a `nil` award_emoji + errors: [] + } + end + end + end +end diff --git a/app/graphql/mutations/award_emojis/toggle.rb b/app/graphql/mutations/award_emojis/toggle.rb new file mode 100644 index 00000000000..c03902e8035 --- /dev/null +++ b/app/graphql/mutations/award_emojis/toggle.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Mutations + module AwardEmojis + class Toggle < Base + graphql_name 'ToggleAwardEmoji' + + field :toggledOn, + GraphQL::BOOLEAN_TYPE, + null: false, + description: 'True when the emoji was awarded, false when it was removed' + + def resolve(args) + awardable = authorized_find!(id: args[:awardable_id]) + + check_object_is_awardable!(awardable) + + # TODO this will be handled by AwardEmoji::ToggleService + # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63372 and + # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29782 + award = awardable.toggle_award_emoji(args[:name], current_user) + + # Destroy returns a collection :( + award = award.first if award.is_a?(Array) + + errors = errors_on_object(award) + + toggled_on = awardable.awarded_emoji?(args[:name], current_user) + + { + # For consistency with the AwardEmojis::Remove mutation, only return + # the AwardEmoji if it was created and not destroyed + award_emoji: (award if toggled_on), + errors: errors, + toggled_on: toggled_on + } + end + end + end +end diff --git a/app/graphql/mutations/base_mutation.rb b/app/graphql/mutations/base_mutation.rb index eb03dfe1624..08d2a1f18a3 100644 --- a/app/graphql/mutations/base_mutation.rb +++ b/app/graphql/mutations/base_mutation.rb @@ -2,6 +2,8 @@ module Mutations class BaseMutation < GraphQL::Schema::RelayClassicMutation + prepend Gitlab::Graphql::CopyFieldDescription + field :errors, [GraphQL::STRING_TYPE], null: false, description: "Reasons why the mutation failed." @@ -9,5 +11,10 @@ module Mutations def current_user context[:current_user] end + + # Returns Array of errors on an ActiveRecord object + def errors_on_object(record) + record.errors.full_messages + end end end diff --git a/app/graphql/types/award_emojis/award_emoji_type.rb b/app/graphql/types/award_emojis/award_emoji_type.rb new file mode 100644 index 00000000000..8daf699a112 --- /dev/null +++ b/app/graphql/types/award_emojis/award_emoji_type.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module Types + module AwardEmojis + class AwardEmojiType < BaseObject + graphql_name 'AwardEmoji' + + authorize :read_emoji + + present_using AwardEmojiPresenter + + field :name, + GraphQL::STRING_TYPE, + null: false, + description: 'The emoji name' + + field :description, + GraphQL::STRING_TYPE, + null: false, + description: 'The emoji description' + + field :unicode, + GraphQL::STRING_TYPE, + null: false, + description: 'The emoji in unicode' + + field :emoji, + GraphQL::STRING_TYPE, + null: false, + description: 'The emoji as an icon' + + field :unicode_version, + GraphQL::STRING_TYPE, + null: false, + description: 'The unicode version for this emoji' + + field :user, + Types::UserType, + null: false, + description: 'The user who awarded the emoji', + resolve: -> (award_emoji, _args, _context) { + Gitlab::Graphql::Loaders::BatchModelLoader.new(User, award_emoji.user_id).find + } + end + end +end diff --git a/app/graphql/types/commit_type.rb b/app/graphql/types/commit_type.rb new file mode 100644 index 00000000000..d73dd73affd --- /dev/null +++ b/app/graphql/types/commit_type.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Types + class CommitType < BaseObject + graphql_name 'Commit' + + authorize :download_code + + present_using CommitPresenter + + field :id, type: GraphQL::ID_TYPE, null: false + field :sha, type: GraphQL::STRING_TYPE, null: false + field :title, type: GraphQL::STRING_TYPE, null: true + field :description, type: GraphQL::STRING_TYPE, null: true + field :message, type: GraphQL::STRING_TYPE, null: true + field :authored_date, type: Types::TimeType, null: true + field :web_url, type: GraphQL::STRING_TYPE, null: false + + # models/commit lazy loads the author by email + field :author, type: Types::UserType, null: true + + field :latest_pipeline, + type: Types::Ci::PipelineType, + null: true, + description: "Latest pipeline for this commit", + resolve: -> (obj, ctx, args) do + Gitlab::Graphql::Loaders::PipelineForShaLoader.new(obj.project, obj.sha).find_last + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index 2b4ef299296..6ef1d816b7c 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -6,6 +6,9 @@ module Types graphql_name "Mutation" + mount_mutation Mutations::AwardEmojis::Add + mount_mutation Mutations::AwardEmojis::Remove + mount_mutation Mutations::AwardEmojis::Toggle mount_mutation Mutations::MergeRequests::SetWip end end diff --git a/app/graphql/types/tree/tree_type.rb b/app/graphql/types/tree/tree_type.rb index 1ee93ed9542..cbc448a0695 100644 --- a/app/graphql/types/tree/tree_type.rb +++ b/app/graphql/types/tree/tree_type.rb @@ -4,6 +4,11 @@ module Types class TreeType < BaseObject graphql_name 'Tree' + # Complexity 10 as it triggers a Gitaly call on each render + field :last_commit, Types::CommitType, null: true, complexity: 10, resolve: -> (tree, args, ctx) do + tree.repository.last_commit_for_path(tree.sha, tree.path) + end + field :trees, Types::Tree::TreeEntryType.connection_type, null: false, resolve: -> (obj, args, ctx) do Gitlab::Graphql::Representation::TreeEntry.decorate(obj.trees, obj.repository) end diff --git a/app/models/board.rb b/app/models/board.rb index e08db764f65..50b6ca9b70f 100644 --- a/app/models/board.rb +++ b/app/models/board.rb @@ -4,11 +4,14 @@ class Board < ApplicationRecord belongs_to :group belongs_to :project - has_many :lists, -> { order(:list_type, :position) }, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent + has_many :lists, -> { ordered }, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent + has_many :destroyable_lists, -> { destroyable.ordered }, class_name: "List" validates :project, presence: true, if: :project_needed? validates :group, presence: true, unless: :project + scope :with_associations, -> { preload(:destroyable_lists) } + def project_needed? !group end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 3727a9861aa..fd5aa216174 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -295,6 +295,11 @@ module Ci end end + def self.latest_for_shas(shas) + max_id_per_sha = for_sha(shas).group(:sha).select("max(id)") + where(id: max_id_per_sha) + end + def self.latest_successful_ids_per_project success.group(:project_id).select('max(id) as id') end diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb index 46d2c345758..22b6b1d720c 100644 --- a/app/models/concerns/relative_positioning.rb +++ b/app/models/concerns/relative_positioning.rb @@ -25,7 +25,7 @@ module RelativePositioning relative_position = position_between(max_relative_position, MAX_POSITION) object.relative_position = relative_position max_relative_position = relative_position - object.save + object.save(touch: false) end end end @@ -159,7 +159,7 @@ module RelativePositioning def save_positionable_neighbours return unless @positionable_neighbours - status = @positionable_neighbours.all?(&:save) + status = @positionable_neighbours.all? { |issue| issue.save(touch: false) } @positionable_neighbours = nil status diff --git a/app/models/list.rb b/app/models/list.rb index 17b1a8510cf..d28a9bda82d 100644 --- a/app/models/list.rb +++ b/app/models/list.rb @@ -16,6 +16,7 @@ class List < ApplicationRecord scope :destroyable, -> { where(list_type: list_types.slice(*destroyable_types).values) } scope :movable, -> { where(list_type: list_types.slice(*movable_types).values) } scope :preload_associations, -> { preload(:board, :label) } + scope :ordered, -> { order(:list_type, :position) } class << self def destroyable_types diff --git a/app/models/snippet.rb b/app/models/snippet.rb index f4fdac2558c..00931457344 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -194,6 +194,10 @@ class Snippet < ApplicationRecord 'snippet' end + def to_ability_name + model_name.singular + end + class << self # Searches for snippets with a matching title or file name. # diff --git a/app/policies/award_emoji_policy.rb b/app/policies/award_emoji_policy.rb new file mode 100644 index 00000000000..21e382e24b3 --- /dev/null +++ b/app/policies/award_emoji_policy.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AwardEmojiPolicy < BasePolicy + delegate { @subject.awardable if DeclarativePolicy.has_policy?(@subject.awardable) } + + condition(:can_read_awardable) do + can?(:"read_#{@subject.awardable.to_ability_name}") + end + + rule { can_read_awardable }.enable :read_emoji +end diff --git a/app/presenters/award_emoji_presenter.rb b/app/presenters/award_emoji_presenter.rb new file mode 100644 index 00000000000..98713855d35 --- /dev/null +++ b/app/presenters/award_emoji_presenter.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class AwardEmojiPresenter < Gitlab::View::Presenter::Delegated + presents :award_emoji + + def description + as_emoji['description'] + end + + def unicode + as_emoji['unicode'] + end + + def emoji + as_emoji['moji'] + end + + def unicode_version + Gitlab::Emoji.emoji_unicode_version(award_emoji.name) + end + + private + + def as_emoji + @emoji ||= Gitlab::Emoji.emojis[award_emoji.name] || {} + end +end diff --git a/app/presenters/commit_presenter.rb b/app/presenters/commit_presenter.rb index 05adbe1d4f5..fc9853733c1 100644 --- a/app/presenters/commit_presenter.rb +++ b/app/presenters/commit_presenter.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true -class CommitPresenter < Gitlab::View::Presenter::Simple +class CommitPresenter < Gitlab::View::Presenter::Delegated + include GlobalID::Identification + presents :commit def status_for(ref) @@ -10,4 +12,8 @@ class CommitPresenter < Gitlab::View::Presenter::Simple def any_pipelines? can?(current_user, :read_pipeline, commit.project) && commit.pipelines.any? end + + def web_url + Gitlab::UrlBuilder.new(commit).url + end end diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb index 43aced598a9..fd2673fa0cc 100644 --- a/app/serializers/merge_request_widget_entity.rb +++ b/app/serializers/merge_request_widget_entity.rb @@ -217,8 +217,12 @@ class MergeRequestWidgetEntity < IssuableEntity project_merge_request_path(merge_request.project, merge_request, format: :diff) end - expose :status_path do |merge_request| - project_merge_request_path(merge_request.target_project, merge_request, format: :json) + expose :merge_request_basic_path do |merge_request| + project_merge_request_path(merge_request.target_project, merge_request, serializer: :basic, format: :json) + end + + expose :merge_request_widget_path do |merge_request| + widget_project_json_merge_request_path(merge_request.target_project, merge_request, format: :json) end expose :ci_environments_status_path do |merge_request| diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 26132f1824a..02de080e0ba 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -205,7 +205,7 @@ class IssuableBaseService < BaseService end if issuable.changed? || params.present? - issuable.assign_attributes(params.merge(updated_by: current_user)) + issuable.assign_attributes(params) if has_title_or_description_changed?(issuable) issuable.assign_attributes(last_edited_at: Time.now, last_edited_by: current_user) @@ -213,11 +213,16 @@ class IssuableBaseService < BaseService before_update(issuable) + # Do not touch when saving the issuable if only changes position within a list. We should call + # this method at this point to capture all possible changes. + should_touch = update_timestamp?(issuable) + + issuable.updated_by = current_user if should_touch # We have to perform this check before saving the issuable as Rails resets # the changed fields upon calling #save. update_project_counters = issuable.project && update_project_counter_caches?(issuable) - if issuable.with_transaction_returning_status { issuable.save } + if issuable.with_transaction_returning_status { issuable.save(touch: should_touch) } # We do not touch as it will affect a update on updated_at field ActiveRecord::Base.no_touching do Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, old_labels: old_associations[:labels]) @@ -402,4 +407,8 @@ class IssuableBaseService < BaseService def ensure_milestone_available(issuable) issuable.milestone_id = nil unless issuable.milestone_available? end + + def update_timestamp?(issuable) + issuable.changes.keys != ["relative_position"] + end end diff --git a/app/services/users/update_service.rb b/app/services/users/update_service.rb index 15c13a452ad..8f52e9cb23f 100644 --- a/app/services/users/update_service.rb +++ b/app/services/users/update_service.rb @@ -63,12 +63,20 @@ module Users def assign_identity return unless identity_params.present? - identity = user.identities.find_or_create_by(provider: identity_params[:provider]) # rubocop: disable CodeReuse/ActiveRecord + identity = user.identities.find_or_create_by(provider_params) # rubocop: disable CodeReuse/ActiveRecord identity.update(identity_params) end def identity_attributes [:provider, :extern_uid] end + + def provider_attributes + [:provider] + end + + def provider_params + identity_params.slice(*provider_attributes) + end end end diff --git a/app/views/projects/_merge_request_settings_description_text.html.haml b/app/views/projects/_merge_request_settings_description_text.html.haml new file mode 100644 index 00000000000..42964c900b3 --- /dev/null +++ b/app/views/projects/_merge_request_settings_description_text.html.haml @@ -0,0 +1 @@ +%p= s_('ProjectSettings|Choose your merge method, merge options, and merge checks.') diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index c15b84d0aac..29b7c45201c 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -27,7 +27,7 @@ .settings-header %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only= _('Merge requests') %button.btn.btn-default.js-settings-toggle{ type: 'button' }= expanded ? _('Collapse') : _('Expand') - %p= _('Choose your merge method, options, checks, and set up a default merge request description template.') + = render_if_exists 'projects/merge_request_settings_description_text' .settings-content = render_if_exists 'shared/promotions/promote_mr_features' diff --git a/app/views/projects/environments/show.html.haml b/app/views/projects/environments/show.html.haml index d59b2d4fb01..c13a47b0b09 100644 --- a/app/views/projects/environments/show.html.haml +++ b/app/views/projects/environments/show.html.haml @@ -31,21 +31,19 @@ = button_to stop_project_environment_path(@project, @environment), class: 'btn btn-danger has-tooltip', method: :post do = s_('Environments|Stop environment') - .row.top-area.adjust - .col-md-7 - %h3.page-title= @environment.name - .col-md-5 - .nav-controls - = render 'projects/environments/terminal_button', environment: @environment - = render 'projects/environments/external_url', environment: @environment - = render 'projects/environments/metrics_button', environment: @environment - - if can?(current_user, :update_environment, @environment) - = link_to _('Edit'), edit_project_environment_path(@project, @environment), class: 'btn' - - if can?(current_user, :stop_environment, @environment) - = button_tag class: 'btn btn-danger', type: 'button', data: { toggle: 'modal', - target: '#stop-environment-modal' } do - = sprite_icon('stop') - = s_('Environments|Stop') + .top-area + %h3.page-title= @environment.name + .nav-controls.ml-auto.my-2 + = render 'projects/environments/terminal_button', environment: @environment + = render 'projects/environments/external_url', environment: @environment + = render 'projects/environments/metrics_button', environment: @environment + - if can?(current_user, :update_environment, @environment) + = link_to _('Edit'), edit_project_environment_path(@project, @environment), class: 'btn' + - if can?(current_user, :stop_environment, @environment) + = button_tag class: 'btn btn-danger', type: 'button', data: { toggle: 'modal', + target: '#stop-environment-modal' } do + = sprite_icon('stop') + = s_('Environments|Stop') .environments-container - if @deployments.blank? diff --git a/app/views/projects/tree/_tree_header.html.haml b/app/views/projects/tree/_tree_header.html.haml index ea6349f2f57..1d0bc588c9c 100644 --- a/app/views/projects/tree/_tree_header.html.haml +++ b/app/views/projects/tree/_tree_header.html.haml @@ -76,6 +76,7 @@ #{ _('New tag') } .tree-controls + = render_if_exists 'projects/tree/lock_link' = link_to s_('Commits|History'), project_commits_path(@project, @id), class: 'btn' = render 'projects/find_file_link' diff --git a/changelogs/unreleased/44949-do-not-update-updated_at-on-an-issue-when-reordering-it.yml b/changelogs/unreleased/44949-do-not-update-updated_at-on-an-issue-when-reordering-it.yml new file mode 100644 index 00000000000..efc6af7845c --- /dev/null +++ b/changelogs/unreleased/44949-do-not-update-updated_at-on-an-issue-when-reordering-it.yml @@ -0,0 +1,5 @@ +--- +title: Will not update issue timestamps when changing positions in a list +merge_request: 29677 +author: +type: changed diff --git a/changelogs/unreleased/58689-regroup-jump-button-in-discussion.yml b/changelogs/unreleased/58689-regroup-jump-button-in-discussion.yml new file mode 100644 index 00000000000..bf6f314f0ce --- /dev/null +++ b/changelogs/unreleased/58689-regroup-jump-button-in-discussion.yml @@ -0,0 +1,6 @@ +--- +title: Improve discussion reply buttons layout and how jump to next discussion button + appears +merge_request: 29779 +author: +type: changed diff --git a/changelogs/unreleased/62826-graphql-emoji-mutations.yml b/changelogs/unreleased/62826-graphql-emoji-mutations.yml new file mode 100644 index 00000000000..0c0aaedf844 --- /dev/null +++ b/changelogs/unreleased/62826-graphql-emoji-mutations.yml @@ -0,0 +1,5 @@ +--- +title: GraphQL mutations for add, remove and toggle emoji +merge_request: 29919 +author: +type: added diff --git a/changelogs/unreleased/62968-environment-details-header-border-misaligned.yml b/changelogs/unreleased/62968-environment-details-header-border-misaligned.yml new file mode 100644 index 00000000000..749fe6a9cb0 --- /dev/null +++ b/changelogs/unreleased/62968-environment-details-header-border-misaligned.yml @@ -0,0 +1,5 @@ +--- +title: Resolve Environment details header border misaligned +merge_request: 30011 +author: +type: fixed diff --git a/changelogs/unreleased/ce-11098-update-merge-request-settings-description-text.yml b/changelogs/unreleased/ce-11098-update-merge-request-settings-description-text.yml new file mode 100644 index 00000000000..9f6a2040095 --- /dev/null +++ b/changelogs/unreleased/ce-11098-update-merge-request-settings-description-text.yml @@ -0,0 +1,5 @@ +--- +title: Update merge requests section description text on project settings page +merge_request: 27838 +author: +type: changed
\ No newline at end of file diff --git a/changelogs/unreleased/graphql-tree-last-commit.yml b/changelogs/unreleased/graphql-tree-last-commit.yml new file mode 100644 index 00000000000..5104ca6687e --- /dev/null +++ b/changelogs/unreleased/graphql-tree-last-commit.yml @@ -0,0 +1,5 @@ +--- +title: Added commit type to tree GraphQL response +merge_request: 29412 +author: +type: added diff --git a/changelogs/unreleased/id-extract-widget-into-different-request.yml b/changelogs/unreleased/id-extract-widget-into-different-request.yml new file mode 100644 index 00000000000..3b9f5fdd6bd --- /dev/null +++ b/changelogs/unreleased/id-extract-widget-into-different-request.yml @@ -0,0 +1,5 @@ +--- +title: Add a separate endpoint for fetching MRs serialized as widgets +merge_request: 29979 +author: +type: performance diff --git a/changelogs/unreleased/sh-support-subnets-ip-rate-limiter.yml b/changelogs/unreleased/sh-support-subnets-ip-rate-limiter.yml new file mode 100644 index 00000000000..3e78c58c764 --- /dev/null +++ b/changelogs/unreleased/sh-support-subnets-ip-rate-limiter.yml @@ -0,0 +1,5 @@ +--- +title: Support CIDR notation in IP rate limiter +merge_request: 30146 +author: +type: changed diff --git a/changelogs/unreleased/support-jsonb-default-value.yml b/changelogs/unreleased/support-jsonb-default-value.yml new file mode 100644 index 00000000000..d46156276f9 --- /dev/null +++ b/changelogs/unreleased/support-jsonb-default-value.yml @@ -0,0 +1,5 @@ +--- +title: Support jsonb default in add_column_with_default migration helper +merge_request: 29871 +author: +type: other diff --git a/config/routes/project.rb b/config/routes/project.rb index bcbbd7222e0..91613e3333f 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -261,6 +261,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do get :commits get :pipelines get :diffs, to: 'merge_requests/diffs#show' + get :widget, to: 'merge_requests/content#widget' end get :diff_for_path, controller: 'merge_requests/diffs' diff --git a/doc/development/contributing/style_guides.md b/doc/development/contributing/style_guides.md index f319d00d7fe..87e61a7476f 100644 --- a/doc/development/contributing/style_guides.md +++ b/doc/development/contributing/style_guides.md @@ -31,8 +31,8 @@ This is also the style used by linting tools such as [Return to Contributing documentation](index.md) -[rss-source]: https://github.com/bbatsov/ruby-style-guide/blob/master/README.md#source-code-layout -[rss-naming]: https://github.com/bbatsov/ruby-style-guide/blob/master/README.md#naming +[rss-source]: https://github.com/rubocop-hq/ruby-style-guide/blob/master/README.adoc#source-code-layout +[rss-naming]: https://github.com/rubocop-hq/ruby-style-guide/blob/master/README.adoc#naming-conventions [doc-guidelines]: ../documentation/index.md "Documentation guidelines" [js-styleguide]: ../fe_guide/style_guide_js.md "JavaScript styleguide" [scss-styleguide]: ../fe_guide/style_guide_scss.md "SCSS styleguide" diff --git a/doc/development/documentation/site_architecture/index.md b/doc/development/documentation/site_architecture/index.md index ee3a9caf9a0..6dd12b5efa7 100644 --- a/doc/development/documentation/site_architecture/index.md +++ b/doc/development/documentation/site_architecture/index.md @@ -11,8 +11,40 @@ and deploy it to <https://docs.gitlab.com>. While the source of the documentation content is stored in GitLab's respective product repositories, the source that is used to build the documentation site _from that content_ -is located at <https://gitlab.com/gitlab-com/gitlab-docs>. See the README there for -detailed information. +is located at <https://gitlab.com/gitlab-com/gitlab-docs>. + +The following diagram illustrates the relationship between the repositories +from where content is sourced, the `gitlab-docs` project, and the published output. + +```mermaid + graph LR + A[gitlab-ce/doc] + B[gitlab-ee/doc] + C[gitlab-runner/docs] + D[omnibus-gitlab/doc] + E[charts/doc] + F[gitlab-docs] + A --> F + B --> F + C --> F + D --> F + E --> F + F -- Build pipeline --> G + G[docs.gitlab.com] + H[/ce/] + I[/ee/] + J[/runner/] + K[/omnibus/] + L[/charts/] + G --> H + G --> I + G --> J + G --> K + G --> L +``` + +See the [README there](https://gitlab.com/gitlab-com/gitlab-docs/blob/master/README.md) +for detailed information. ## Assets @@ -22,9 +54,9 @@ the GitLab Documentation website. ### Libraries -- [Bootstrap 3.3 components](https://getbootstrap.com/docs/3.3/components/) -- [Bootstrap 3.3 JS](https://getbootstrap.com/docs/3.3/javascript/) -- [jQuery](https://jquery.com/) 3.2.1 +- [Bootstrap 4.3.1 components](https://getbootstrap.com/docs/4.3/components/) +- [Bootstrap 4.3.1 JS](https://getbootstrap.com/docs/4.3/getting-started/javascript/) +- [jQuery](https://jquery.com/) 3.3.1 - [Clipboard JS](https://clipboardjs.com/) - [Font Awesome 4.7.0](https://fontawesome.com/v4.7.0/icons/) diff --git a/doc/security/rack_attack.md b/doc/security/rack_attack.md index fa4b0d1fb09..8695b5d2194 100644 --- a/doc/security/rack_attack.md +++ b/doc/security/rack_attack.md @@ -53,8 +53,9 @@ For more information on how to use these options check out The following settings can be configured: - `enabled`: By default this is set to `false`. Set this to `true` to enable Rack Attack. -- `ip_whitelist`: Whitelist any IPs from being blocked. They must be formatted as strings within a ruby array. - For example, `["127.0.0.1", "127.0.0.2", "127.0.0.3"]`. +- `ip_whitelist`: Whitelist any IPs from being blocked. They must be formatted as strings within a Ruby array. + CIDR notation is supported in GitLab v12.1 and up. + For example, `["127.0.0.1", "127.0.0.2", "127.0.0.3", "192.168.0.1/24"]`. - `maxretry`: The maximum amount of times a request can be made in the specified time. - `findtime`: The maximum amount of time that failed requests can count against an IP diff --git a/doc/user/admin_area/index.md b/doc/user/admin_area/index.md index fa60ee96cf7..d2947ae3371 100644 --- a/doc/user/admin_area/index.md +++ b/doc/user/admin_area/index.md @@ -21,7 +21,7 @@ The Admin Area is made up of the following sections: | Section | Description | |:------------------------------|:-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | [Overview](#overview-section) | View your GitLab [Dashboard](#admin-dashboard), and administer [projects](#administering-projects), [users](#administering-users), [groups](#administering-groups), [jobs](#administering-jobs), [Runners](#administering-runners), and [Gitaly servers](#administering-gitaly-servers). | -| Monitoring | View GitLab system information, and information on background jobs, logs, [health checks](monitoring/health_check.md), request profiles, and audit logs. | +| Monitoring | View GitLab [system information](#system-info), and information on [background jobs](#background-jobs), [logs](#logs), [health checks](monitoring/health_check.md), [requests profiles](#requests-profiles), and [audit logs](#audit-log-premium-only). | | Messages | Send and manage [broadcast messages](broadcast_messages.md) for your users. | | System Hooks | Configure [system hooks](../../system_hooks/system_hooks.md) for many events. | | Applications | Create system [OAuth applications](../../integration/oauth_provider.md) for integrations with other services. | @@ -229,3 +229,66 @@ For each Gitaly server, the following details are listed: | Server version | Gitaly version | | Git version | Version of Git installed on the Gitaly server | | Up to date | Indicates if the Gitaly server version is the latest version available. A green dot indicates the server is up to date. | + +## Monitoring section + +The following topics document the **Monitoring** section of the Admin Area. + +### System Info + +The **System Info** page provides the following statistics: + +| Field | Description | +| :----------- | :---------- | +| CPU | Number of CPU cores available | +| Memory Usage | Memory in use, and total memory available | +| Disk Usage | Disk space in use, and total disk space available | +| Uptime | Approximate uptime of the GitLab instance | + +These statistics are updated only when you navigate to the **System Info** page, or you refresh the page in your browser. + +### Background Jobs + +The **Background Jobs** page displays the Sidekiq dashboard. Sidekiq is used by GitLab to +perform processing in the background. + +The Sidekiq dashboard consists of the following elements: + +- A tab per jobs' status. +- A breakdown of background job statistics. +- A live graph of **Processed** and **Failed** jobs, with a selectable polling interval. +- An historical graph of **Processed** and **Failed** jobs, with a selectable time span. +- Redis statistics, including: + - Version number + - Uptime, measured in days + - Number of connections + - Current memory usage, measured in MB + - Peak memory usage, measured in MB + +### Logs + +The **Logs** page provides access to the following log files: + +| Log file | Contents | +| :---------------------- | :------- | +| `application.log` | GitLab user activity | +| `githost.log` | Failed GitLab interaction with Git repositories | +| `production.log` | Requests received from Unicorn, and the actions taken to serve those requests | +| `sidekiq.log` | Background jobs | +| `repocheck.log` | Repository activity | +| `integrations_json.log` | Activity between GitLab and integrated systems | +| `kubernetes.log` | Kubernetes activity | + +The contents of these log files can be useful when troubleshooting a problem. Access is available to GitLab admins, without requiring direct access to the log files. + +For details of these log files and their contents, see [Log system](../../administration/logs.md). + +The content of each log file is listed in chronological order. To minimize performance issues, a maximum 2000 lines of each log file are shown. + +### Requests Profiles + +The **Requests Profiles** page contains the token required for profiling. For more details, see [Request Profiling](../../administration/monitoring/performance/request_profiling.md). + +### Audit Log **[PREMIUM ONLY]** + +The **Audit Log** page lists changes made within the GitLab server. With this information you can control, analyze, and track every change. diff --git a/doc/user/application_security/container_scanning/index.md b/doc/user/application_security/container_scanning/index.md index 4a2fb1d7190..9dfbe326f1d 100644 --- a/doc/user/application_security/container_scanning/index.md +++ b/doc/user/application_security/container_scanning/index.md @@ -206,6 +206,11 @@ vulnerabilities in your groups and projects. Read more about the Once a vulnerability is found, you can interact with it. Read more on how to [interact with the vulnerabilities](../index.md#interacting-with-the-vulnerabilities). +## Vulnerabilities database update + +For more information about the vulnerabilities database update, check the +[maintenance table](../index.md#maintenance-and-update-of-the-vulnerabilities-database). + ## Troubleshooting ### docker: Error response from daemon: failed to copy xattrs diff --git a/doc/user/application_security/dast/index.md b/doc/user/application_security/dast/index.md index a722aa88f9d..2283efe3a44 100644 --- a/doc/user/application_security/dast/index.md +++ b/doc/user/application_security/dast/index.md @@ -259,3 +259,8 @@ vulnerabilities in your groups and projects. Read more about the Once a vulnerability is found, you can interact with it. Read more on how to [interact with the vulnerabilities](../index.md#interacting-with-the-vulnerabilities). + +## Vulnerabilities database update + +For more information about the vulnerabilities database update, check the +[maintenance table](../index.md#maintenance-and-update-of-the-vulnerabilities-database). diff --git a/doc/user/application_security/dependency_scanning/index.md b/doc/user/application_security/dependency_scanning/index.md index ea8b96eb24d..9145e034dcb 100644 --- a/doc/user/application_security/dependency_scanning/index.md +++ b/doc/user/application_security/dependency_scanning/index.md @@ -404,6 +404,11 @@ vulnerabilities in your groups and projects. Read more about the Once a vulnerability is found, you can interact with it. Read more on how to [interact with the vulnerabilities](../index.md#interacting-with-the-vulnerabilities). +## Vulnerabilities database update + +For more information about the vulnerabilities database update, check the +[maintenance table](../index.md#maintenance-and-update-of-the-vulnerabilities-database). + ## Dependency List > [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/10075) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 12.0. diff --git a/doc/user/application_security/index.md b/doc/user/application_security/index.md index 679847b76d7..69fa1ec5da6 100644 --- a/doc/user/application_security/index.md +++ b/doc/user/application_security/index.md @@ -10,7 +10,7 @@ high-level view on projects and groups, and start remediation processes when nee GitLab can scan and report any vulnerabilities found in your project. -| Secure scanning tools | Description | +| Secure scanning tool | Description | |:-----------------------------------------------------------------------------|:-----------------------------------------------------------------------| | [Container Scanning](container_scanning/index.md) **[ULTIMATE]** | Scan Docker containers for known vulnerabilities. | | [Dependency Scanning](dependency_scanning/index.md) **[ULTIMATE]** | Analyze your dependencies for known vulnerabilities. | @@ -19,6 +19,29 @@ GitLab can scan and report any vulnerabilities found in your project. | [Security Dashboard](security_dashboard/index.md) **[ULTIMATE]** | View vulnerabilities in all your projects and groups. | | [Static Application Security Testing (SAST)](sast/index.md) **[ULTIMATE]** | Analyze source code for known vulnerabilities. | +## Maintenance and update of the vulnerabilities database + +The various scanning tools and the vulnerabilities database are updated regularly. + +| Secure scanning tool | Vulnerabilities database updates | +|:-------------------------------------------------------------|-------------------------------------------| +| [Container Scanning](container_scanning/index.md) | Uses `clair` underneath and the latest `clair-db` version is used for each job run by running the [`latest` docker image tag](https://gitlab.com/gitlab-org/gitlab-ee/blob/438a0a56dc0882f22bdd82e700554525f552d91b/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml#L37). The `clair-db` database [is updated daily according to the author](https://github.com/arminc/clair-local-scan#clair-server-or-local). | +| [Dependency Scanning](dependency_scanning/index.md) | Relies on `bundler-audit` (for Rubygems), `retire.js` (for NPM packages) and `gemnasium` (GitLab's own tool for all libraries). `bundler-audit` and `retire.js` both fetch their vulnerabilities data from GitHub repositories, so vulnerabilities added to `ruby-advisory-db` and `retire.js` are immediately available. The tools themselves are updated once per month if there's a new version. The [Gemnasium DB](https://gitlab.com/gitlab-org/security-products/gemnasium-db) is updated at least once a week. | +| [Dynamic Application Security Testing (DAST)](dast/index.md) | Updated weekly on Sundays. The underlying tool, `zaproxy`, downloads fresh rules at startup. | +| [Static Application Security Testing (SAST)](sast/index.md) | Relies exclusively on [the tools GitLab is wrapping](sast/index.md#supported-languages-and-frameworks). The underlying analyzers are updated at least once per month if a relevant update is available. The vulnerabilities database is updated by the upstream tools. | + +You don't have to update GitLab to benefit from the latest vulnerabilities definitions, +but you may have to in the future. + +The security tools are released as Docker images, and the vendored job definitions +to enable them are using the `x-y-stable` image tags that get overridden each time a new +release of the tools is pushed. The Docker images are updated to match the +previous GitLab releases, so they automatically get the latest versions of the +scanning tools without the user having to do anything. + +This workflow comes with some drawbacks and there's a +[plan to change this](https://gitlab.com/gitlab-org/gitlab-ee/issues/9725). + ## Interacting with the vulnerabilities > Introduced in [GitLab Ultimate](https://about.gitlab.com/pricing) 10.8. diff --git a/doc/user/application_security/sast/index.md b/doc/user/application_security/sast/index.md index ec3f7fbde76..9074ac3f4a1 100644 --- a/doc/user/application_security/sast/index.md +++ b/doc/user/application_security/sast/index.md @@ -269,7 +269,7 @@ it highlighted: "url": "https://cwe.mitre.org/data/definitions/330.html" } ] - }, + }, { "category": "sast", "message": "Probable insecure usage of temp file/directory.", @@ -296,7 +296,7 @@ it highlighted: "url": "https://docs.openstack.org/bandit/latest/plugins/b108_hardcoded_tmp_directory.html" } ] - }, + }, ], "remediations": [] } @@ -320,7 +320,7 @@ the report JSON unless stated otherwise. Presence of optional fields depends on | `vulnerabilities[].scanner` | A node that describes the analyzer used to find this vulnerability. | | `vulnerabilities[].scanner.id` | Id of the scanner as a snake_case string. | | `vulnerabilities[].scanner.name` | Name of the scanner, for display purposes. | -| `vulnerabilities[].location` | A node that tells where the vulnerability is located. | +| `vulnerabilities[].location` | A node that tells where the vulnerability is located. | | `vulnerabilities[].location.file` | Path to the file where the vulnerability is located. Optional. | | `vulnerabilities[].location.start_line` | The first line of the code affected by the vulnerability. Optional. | | `vulnerabilities[].location.end_line` | The last line of the code affected by the vulnerability. Optional. | @@ -330,7 +330,7 @@ the report JSON unless stated otherwise. Presence of optional fields depends on | `vulnerabilities[].identifiers[].type` | Type of the identifier. Possible values: common identifier types (among `cve`, `cwe`, `osvdb`, and `usn`) or analyzer-dependent ones (e.g., `bandit_test_id` for [Bandit analyzer](https://wiki.openstack.org/wiki/Security/Projects/Bandit)). | | `vulnerabilities[].identifiers[].name` | Name of the identifier for display purposes. | | `vulnerabilities[].identifiers[].value` | Value of the identifier for matching purposes. | -| `vulnerabilities[].identifiers[].url` | URL to identifier's documentation. Optional. | +| `vulnerabilities[].identifiers[].url` | URL to identifier's documentation. Optional. | ## Secret detection @@ -363,3 +363,8 @@ vulnerabilities in your groups and projects. Read more about the Once a vulnerability is found, you can interact with it. Read more on how to [interact with the vulnerabilities](../index.md#interacting-with-the-vulnerabilities). + +## Vulnerabilities database update + +For more information about the vulnerabilities database update, check the +[maintenance table](../index.md#maintenance-and-update-of-the-vulnerabilities-database). diff --git a/doc/user/project/clusters/index.md b/doc/user/project/clusters/index.md index 97d2dfc0f7e..c6ee168bad0 100644 --- a/doc/user/project/clusters/index.md +++ b/doc/user/project/clusters/index.md @@ -533,22 +533,20 @@ This job failed because the necessary resources were not successfully created. To find the cause of this error when creating a namespace and service account, check the [logs](../../../administration/logs.md#kuberneteslog). -NOTE: **NOTE:** -As of GitLab 12.1 we require [`cluster-admin`](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#user-facing-roles) -tokens for all project level clusters unless you unselect the -[GitLab-managed cluster](#gitlab-managed-clusters) option. If you -want to manage namespaces and service accounts yourself and don't -want to provide a `cluster-admin` token to GitLab you must unselect this -option or you will get the above error. - -Common reasons for failure include: +Reasons for failure include: -- The token you gave GitLab did not have [`cluster-admin`](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#user-facing-roles) +- The token you gave GitLab does not have [`cluster-admin`](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#user-facing-roles) privileges required by GitLab. - Missing `KUBECONFIG` or `KUBE_TOKEN` variables. To be passed to your job, they must have a matching [`environment:name`](../../../ci/environments.md#defining-environments). If your job has no `environment:name` set, it will not be passed the Kubernetes credentials. +NOTE: **NOTE:** +Project-level clusters upgraded from GitLab 12.0 or older may be configured +in a way that causes this error. Ensure you deselect the +[GitLab-managed cluster](#gitlab-managed-clusters) option if you want to manage +namespaces and service accounts yourself. + ## Monitoring your Kubernetes cluster **[ULTIMATE]** > [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/4701) in [GitLab Ultimate][ee] 10.6. diff --git a/lib/api/boards.rb b/lib/api/boards.rb index b7c77730afb..4e31f74f18a 100644 --- a/lib/api/boards.rb +++ b/lib/api/boards.rb @@ -27,7 +27,7 @@ module API end get '/' do authorize!(:read_board, user_project) - present paginate(board_parent.boards), with: Entities::Board + present paginate(board_parent.boards.with_associations), with: Entities::Board end desc 'Find a project board' do diff --git a/lib/api/boards_responses.rb b/lib/api/boards_responses.rb index 86d9b24802f..68497a08fb8 100644 --- a/lib/api/boards_responses.rb +++ b/lib/api/boards_responses.rb @@ -11,7 +11,7 @@ module API end def board_lists - board.lists.destroyable + board.destroyable_lists end def create_list diff --git a/lib/api/entities.rb b/lib/api/entities.rb index ead01dc53f7..d783591c238 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1101,7 +1101,7 @@ module API expose :project, using: Entities::BasicProjectDetails expose :lists, using: Entities::List do |board| - board.lists.destroyable + board.destroyable_lists end end diff --git a/lib/api/group_boards.rb b/lib/api/group_boards.rb index 9a20ee8c8b9..feb2254963e 100644 --- a/lib/api/group_boards.rb +++ b/lib/api/group_boards.rb @@ -37,7 +37,7 @@ module API use :pagination end get '/' do - present paginate(board_parent.boards), with: Entities::Board + present paginate(board_parent.boards.with_associations), with: Entities::Board end end diff --git a/lib/gitlab/auth/ip_rate_limiter.rb b/lib/gitlab/auth/ip_rate_limiter.rb index 81e616fa20a..0b7055b3256 100644 --- a/lib/gitlab/auth/ip_rate_limiter.rb +++ b/lib/gitlab/auth/ip_rate_limiter.rb @@ -3,6 +3,8 @@ module Gitlab module Auth class IpRateLimiter + include ::Gitlab::Utils::StrongMemoize + attr_reader :ip def initialize(ip) @@ -37,7 +39,20 @@ module Gitlab end def ip_can_be_banned? - config.ip_whitelist.exclude?(ip) + !trusted_ip? + end + + def trusted_ip? + trusted_ips.any? { |netmask| netmask.include?(ip) } + end + + def trusted_ips + strong_memoize(:trusted_ips) do + config.ip_whitelist.map do |proxy| + IPAddr.new(proxy) + rescue IPAddr::InvalidAddressError + end.compact + end end end end diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 0b12e862ded..e2cbf91f281 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -434,7 +434,8 @@ module Gitlab end begin - update_column_in_batches(table, column, default, &block) + default_after_type_cast = connection.type_cast(default, column_for(table, column)) + update_column_in_batches(table, column, default_after_type_cast, &block) change_column_null(table, column, false) unless allow_null # We want to rescue _all_ exceptions here, even those that don't inherit diff --git a/lib/gitlab/graphql/copy_field_description.rb b/lib/gitlab/graphql/copy_field_description.rb new file mode 100644 index 00000000000..edd73083ff2 --- /dev/null +++ b/lib/gitlab/graphql/copy_field_description.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module CopyFieldDescription + extend ActiveSupport::Concern + + class_methods do + # Returns the `description` for property of field `field_name` on type. + # This can be used to ensure, for example, that mutation argument descriptions + # are always identical to the corresponding query field descriptions. + # + # E.g.: + # argument :name, GraphQL::STRING_TYPE, description: copy_field_description(Types::UserType, :name) + def copy_field_description(type, field_name) + type.fields[field_name.to_s.camelize(:lower)].description + end + end + end + end +end diff --git a/lib/gitlab/graphql/errors.rb b/lib/gitlab/graphql/errors.rb index fe74549e322..40b90310e8b 100644 --- a/lib/gitlab/graphql/errors.rb +++ b/lib/gitlab/graphql/errors.rb @@ -6,6 +6,7 @@ module Gitlab BaseError = Class.new(GraphQL::ExecutionError) ArgumentError = Class.new(BaseError) ResourceNotAvailable = Class.new(BaseError) + MutationError = Class.new(BaseError) end end end diff --git a/lib/gitlab/graphql/loaders/pipeline_for_sha_loader.rb b/lib/gitlab/graphql/loaders/pipeline_for_sha_loader.rb new file mode 100644 index 00000000000..81c5cabf451 --- /dev/null +++ b/lib/gitlab/graphql/loaders/pipeline_for_sha_loader.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module Loaders + class PipelineForShaLoader + attr_accessor :project, :sha + + def initialize(project, sha) + @project, @sha = project, sha + end + + def find_last + BatchLoader.for(sha).batch(key: project) do |shas, loader, args| + pipelines = args[:key].ci_pipelines.latest_for_shas(shas) + + pipelines.each do |pipeline| + loader.call(pipeline.sha, pipeline) + end + end + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9ea368816f9..b8ce2c20563 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1991,9 +1991,6 @@ msgstr "" msgid "Choose visibility level, enable/disable project features (issues, repository, wiki, snippets) and set permissions." msgstr "" -msgid "Choose your merge method, options, checks, and set up a default merge request description template." -msgstr "" - msgid "CiStatusLabel|canceled" msgstr "" @@ -6652,6 +6649,9 @@ msgstr "" msgid "No" msgstr "" +msgid "No %{header} for this request." +msgstr "" + msgid "No %{providerTitle} repositories available to import" msgstr "" @@ -7137,6 +7137,18 @@ msgstr "" msgid "Performance optimization" msgstr "" +msgid "PerformanceBar|Gitaly calls" +msgstr "" + +msgid "PerformanceBar|SQL queries" +msgstr "" + +msgid "PerformanceBar|profile" +msgstr "" + +msgid "PerformanceBar|trace" +msgstr "" + msgid "Permissions" msgstr "" @@ -8058,6 +8070,9 @@ msgstr "" msgid "ProjectSettings|Badges" msgstr "" +msgid "ProjectSettings|Choose your merge method, merge options, and merge checks." +msgstr "" + msgid "ProjectSettings|Customize your project badges." msgstr "" diff --git a/spec/controllers/projects/merge_requests/content_controller_spec.rb b/spec/controllers/projects/merge_requests/content_controller_spec.rb new file mode 100644 index 00000000000..2879e06aee4 --- /dev/null +++ b/spec/controllers/projects/merge_requests/content_controller_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::MergeRequests::ContentController do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request, target_project: project, source_project: project) } + + before do + sign_in(user) + end + + def do_request + get :widget, params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: merge_request.iid, + format: :json + } + end + + describe 'GET widget' do + context 'user has access to the project' do + before do + expect(::Gitlab::GitalyClient).to receive(:allow_ref_name_caching).and_call_original + + project.add_maintainer(user) + end + + it 'renders widget MR entity as json' do + do_request + + expect(response).to match_response_schema('entities/merge_request_widget') + end + + it 'checks whether the MR can be merged' do + controller.instance_variable_set(:@merge_request, merge_request) + + expect(merge_request).to receive(:check_mergeability) + + do_request + end + + it 'closes an MR with moved source project' do + merge_request.update_column(:source_project_id, nil) + + expect { do_request }.to change { merge_request.reload.open? }.from(true).to(false) + end + end + + context 'user does not have access to the project' do + it 'renders widget MR entity as json' do + do_request + + expect(response).to have_http_status(:not_found) + end + end + end +end diff --git a/spec/factories/award_emoji.rb b/spec/factories/award_emoji.rb index d37e2bf511e..43753fa650c 100644 --- a/spec/factories/award_emoji.rb +++ b/spec/factories/award_emoji.rb @@ -5,7 +5,7 @@ FactoryBot.define do awardable factory: :issue after(:create) do |award, evaluator| - award.awardable.project.add_guest(evaluator.user) + award.awardable.project&.add_guest(evaluator.user) end trait :upvote diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb index 08fa4a98feb..260eec7a9ed 100644 --- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb +++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb @@ -362,14 +362,14 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end end - it 'shows jump to next discussion button except on last discussion' do + it 'shows jump to next discussion button on all discussions' do wait_for_requests all_discussion_replies = page.all('.discussion-reply-holder') expect(all_discussion_replies.count).to eq(2) expect(all_discussion_replies.first.all('.discussion-next-btn').count).to eq(1) - expect(all_discussion_replies.last.all('.discussion-next-btn').count).to eq(0) + expect(all_discussion_replies.last.all('.discussion-next-btn').count).to eq(1) end it 'displays next discussion even if hidden' do diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index 7018cb9a305..eac1dbc6474 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -99,7 +99,8 @@ "revert_in_fork_path": { "type": ["string", "null"] }, "email_patches_path": { "type": "string" }, "plain_diff_path": { "type": "string" }, - "status_path": { "type": "string" }, + "merge_request_basic_path": { "type": "string" }, + "merge_request_widget_path": { "type": "string" }, "new_blob_path": { "type": ["string", "null"] }, "merge_check_path": { "type": "string" }, "ci_environments_status_path": { "type": "string" }, diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb index 4076c1f824b..d36e428a8ee 100644 --- a/spec/graphql/gitlab_schema_spec.rb +++ b/spec/graphql/gitlab_schema_spec.rb @@ -113,7 +113,7 @@ describe GitlabSchema do end it "raises a meaningful error if a global id couldn't be generated" do - expect { described_class.id_from_object(build(:commit)) } + expect { described_class.id_from_object(build(:wiki_directory)) } .to raise_error(RuntimeError, /include `GlobalID::Identification` into/i) end end diff --git a/spec/graphql/types/award_emojis/award_emoji_type_spec.rb b/spec/graphql/types/award_emojis/award_emoji_type_spec.rb new file mode 100644 index 00000000000..5663a3d7195 --- /dev/null +++ b/spec/graphql/types/award_emojis/award_emoji_type_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GitlabSchema.types['AwardEmoji'] do + it { expect(described_class.graphql_name).to eq('AwardEmoji') } + + it { is_expected.to require_graphql_authorizations(:read_emoji) } + + it { expect(described_class).to have_graphql_fields(:description, :unicode_version, :emoji, :name, :unicode, :user) } +end diff --git a/spec/graphql/types/commit_type_spec.rb b/spec/graphql/types/commit_type_spec.rb new file mode 100644 index 00000000000..5d8edcf254c --- /dev/null +++ b/spec/graphql/types/commit_type_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GitlabSchema.types['Commit'] do + it { expect(described_class.graphql_name).to eq('Commit') } + + it { expect(described_class).to require_graphql_authorizations(:download_code) } + + it { expect(described_class).to have_graphql_fields(:id, :sha, :title, :description, :message, :authored_date, :author, :web_url, :latest_pipeline) } +end diff --git a/spec/graphql/types/tree/tree_type_spec.rb b/spec/graphql/types/tree/tree_type_spec.rb index b9c5570115e..23779d75600 100644 --- a/spec/graphql/types/tree/tree_type_spec.rb +++ b/spec/graphql/types/tree/tree_type_spec.rb @@ -5,5 +5,5 @@ require 'spec_helper' describe Types::Tree::TreeType do it { expect(described_class.graphql_name).to eq('Tree') } - it { expect(described_class).to have_graphql_fields(:trees, :submodules, :blobs) } + it { expect(described_class).to have_graphql_fields(:trees, :submodules, :blobs, :last_commit) } end diff --git a/spec/javascripts/notes/stores/getters_spec.js b/spec/javascripts/notes/stores/getters_spec.js index 8f3c493dd4c..c3ed079e33b 100644 --- a/spec/javascripts/notes/stores/getters_spec.js +++ b/spec/javascripts/notes/stores/getters_spec.js @@ -32,6 +32,26 @@ describe('Getters Notes Store', () => { }; }); + describe('showJumpToNextDiscussion', () => { + it('should return true if there are 2 or more unresolved discussions', () => { + const localGetters = { + unresolvedDiscussionsIdsByDate: ['123', '456'], + allResolvableDiscussions: [], + }; + + expect(getters.showJumpToNextDiscussion(state, localGetters)()).toBe(true); + }); + + it('should return false if there are 1 or less unresolved discussions', () => { + const localGetters = { + unresolvedDiscussionsIdsByDate: ['123'], + allResolvableDiscussions: [], + }; + + expect(getters.showJumpToNextDiscussion(state, localGetters)()).toBe(false); + }); + }); + describe('discussions', () => { it('should return all discussions in the store', () => { expect(getters.discussions(state)).toEqual([individualNote]); diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js index 48f812f0db4..253413ae43e 100644 --- a/spec/javascripts/vue_mr_widget/mock_data.js +++ b/spec/javascripts/vue_mr_widget/mock_data.js @@ -218,7 +218,8 @@ export default { '/root/acets-app/forks?continue%5Bnotice%5D=You%27re+not+allowed+to+make+changes+to+this+project+directly.+A+fork+of+this+project+has+been+created+that+you+can+make+changes+in%2C+so+you+can+submit+a+merge+request.+Try+to+cherry-pick+this+commit+again.&continue%5Bnotice_now%5D=You%27re+not+allowed+to+make+changes+to+this+project+directly.+A+fork+of+this+project+is+being+created+that+you+can+make+changes+in%2C+so+you+can+submit+a+merge+request.&continue%5Bto%5D=%2Froot%2Facets-app%2Fmerge_requests%2F22&namespace_key=1', email_patches_path: '/root/acets-app/merge_requests/22.patch', plain_diff_path: '/root/acets-app/merge_requests/22.diff', - status_path: '/root/acets-app/merge_requests/22.json', + merge_request_basic_path: '/root/acets-app/merge_requests/22.json?serializer=basic', + merge_request_widget_path: '/root/acets-app/merge_requests/22/widget.json', merge_check_path: '/root/acets-app/merge_requests/22/merge_check', ci_environments_status_url: '/root/acets-app/merge_requests/22/ci_environments_status', project_archived: false, diff --git a/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb b/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb new file mode 100644 index 00000000000..8d6bf45ab30 --- /dev/null +++ b/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Auth::IpRateLimiter, :use_clean_rails_memory_store_caching do + let(:ip) { '10.2.2.3' } + let(:whitelist) { ['127.0.0.1'] } + let(:options) do + { + enabled: true, + ip_whitelist: whitelist, + bantime: 1.minute, + findtime: 1.minute, + maxretry: 2 + } + end + + subject { described_class.new(ip) } + + before do + stub_rack_attack_setting(options) + end + + after do + subject.reset! + end + + describe '#register_fail!' do + it 'bans after 3 consecutive failures' do + expect(subject.banned?).to be_falsey + + 3.times { subject.register_fail! } + + expect(subject.banned?).to be_truthy + end + + shared_examples 'whitelisted IPs' do + it 'does not ban after max retry limit' do + expect(subject.banned?).to be_falsey + + 3.times { subject.register_fail! } + + expect(subject.banned?).to be_falsey + end + end + + context 'with a whitelisted netmask' do + before do + options[:ip_whitelist] = ['127.0.0.1', '10.2.2.0/24', 'bad'] + stub_rack_attack_setting(options) + end + + it_behaves_like 'whitelisted IPs' + end + + context 'with a whitelisted IP' do + before do + options[:ip_whitelist] = ['10.2.2.3'] + stub_rack_attack_setting(options) + end + + it_behaves_like 'whitelisted IPs' + end + end +end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 3cf3d032bf4..7409572288c 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -583,6 +583,24 @@ describe Gitlab::Database::MigrationHelpers do model.add_column_with_default(:projects, :foo, :integer, default: 10, limit: 8) end end + + it 'adds a column with an array default value for a jsonb type' do + create(:project) + allow(model).to receive(:transaction_open?).and_return(false) + allow(model).to receive(:transaction).and_yield + expect(model).to receive(:update_column_in_batches).with(:projects, :foo, '[{"foo":"json"}]').and_call_original + + model.add_column_with_default(:projects, :foo, :jsonb, default: [{ foo: "json" }]) + end + + it 'adds a column with an object default value for a jsonb type' do + create(:project) + allow(model).to receive(:transaction_open?).and_return(false) + allow(model).to receive(:transaction).and_yield + expect(model).to receive(:update_column_in_batches).with(:projects, :foo, '{"foo":"json"}').and_call_original + + model.add_column_with_default(:projects, :foo, :jsonb, default: { foo: "json" }) + end end context 'inside a transaction' do diff --git a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb index 20842f55014..50138d272c4 100644 --- a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb +++ b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb @@ -67,7 +67,7 @@ describe Gitlab::Graphql::Authorize::AuthorizeResource do end describe '#authorize!' do - it 'does not raise an error' do + it 'raises an error' do expect { loading_resource.authorize!(project) }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) end end diff --git a/spec/lib/gitlab/graphql/copy_field_description_spec.rb b/spec/lib/gitlab/graphql/copy_field_description_spec.rb new file mode 100644 index 00000000000..e7462c5b954 --- /dev/null +++ b/spec/lib/gitlab/graphql/copy_field_description_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Graphql::CopyFieldDescription do + subject { Class.new.include(described_class) } + + describe '.copy_field_description' do + let(:type) do + Class.new(Types::BaseObject) do + graphql_name "TestType" + + field :field_name, GraphQL::STRING_TYPE, null: true, description: 'Foo' + end + end + + it 'returns the correct description' do + expect(subject.copy_field_description(type, :field_name)).to eq('Foo') + end + end +end diff --git a/spec/lib/gitlab/graphql/loaders/pipeline_for_sha_loader_spec.rb b/spec/lib/gitlab/graphql/loaders/pipeline_for_sha_loader_spec.rb new file mode 100644 index 00000000000..927476cc655 --- /dev/null +++ b/spec/lib/gitlab/graphql/loaders/pipeline_for_sha_loader_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe Gitlab::Graphql::Loaders::PipelineForShaLoader do + include GraphqlHelpers + + describe '#find_last' do + it 'batch-resolves latest pipeline' do + project = create(:project, :repository) + pipeline1 = create(:ci_pipeline, project: project, ref: project.default_branch, sha: project.commit.sha) + pipeline2 = create(:ci_pipeline, project: project, ref: project.default_branch, sha: project.commit.sha) + pipeline3 = create(:ci_pipeline, project: project, ref: 'improve/awesome', sha: project.commit('improve/awesome').sha) + + result = batch(max_queries: 1) do + [pipeline1.sha, pipeline3.sha].map { |sha| described_class.new(project, sha).find_last } + end + + expect(result).to contain_exactly(pipeline2, pipeline3) + end + end +end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 6ebc6337d50..55cea48b641 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1886,6 +1886,17 @@ describe Ci::Pipeline, :mailer do end end + describe '.latest_for_shas' do + let(:sha) { 'abc' } + + it 'returns latest pipeline for sha' do + create(:ci_pipeline, sha: sha) + pipeline2 = create(:ci_pipeline, sha: sha) + + expect(described_class.latest_for_shas(sha)).to contain_exactly(pipeline2) + end + end + describe '.latest_successful_ids_per_project' do let(:projects) { create_list(:project, 2) } let!(:pipeline1) { create(:ci_pipeline, :success, project: projects[0]) } diff --git a/spec/policies/award_emoji_policy_spec.rb b/spec/policies/award_emoji_policy_spec.rb new file mode 100644 index 00000000000..2e3693c58d7 --- /dev/null +++ b/spec/policies/award_emoji_policy_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AwardEmojiPolicy do + let(:user) { create(:user) } + let(:award_emoji) { create(:award_emoji, awardable: awardable) } + + subject { described_class.new(user, award_emoji) } + + shared_examples 'when the user can read the awardable' do + context do + let(:project) { create(:project, :public) } + + it { expect_allowed(:read_emoji) } + end + end + + shared_examples 'when the user cannot read the awardable' do + context do + let(:project) { create(:project, :private) } + + it { expect_disallowed(:read_emoji) } + end + end + + context 'when the awardable is an issue' do + let(:awardable) { create(:issue, project: project) } + + include_examples 'when the user can read the awardable' + include_examples 'when the user cannot read the awardable' + end + + context 'when the awardable is a merge request' do + let(:awardable) { create(:merge_request, source_project: project) } + + include_examples 'when the user can read the awardable' + include_examples 'when the user cannot read the awardable' + end + + context 'when the awardable is a note' do + let(:awardable) { create(:note_on_merge_request, project: project) } + + include_examples 'when the user can read the awardable' + include_examples 'when the user cannot read the awardable' + end + + context 'when the awardable is a snippet' do + let(:awardable) { create(:project_snippet, :public, project: project) } + + include_examples 'when the user can read the awardable' + include_examples 'when the user cannot read the awardable' + end +end diff --git a/spec/presenters/award_emoji_presenter_spec.rb b/spec/presenters/award_emoji_presenter_spec.rb new file mode 100644 index 00000000000..e2ada2a3c93 --- /dev/null +++ b/spec/presenters/award_emoji_presenter_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe AwardEmojiPresenter do + let(:emoji_name) { 'thumbsup' } + let(:award_emoji) { build(:award_emoji, name: emoji_name) } + let(:presenter) { described_class.new(award_emoji) } + + describe '#description' do + it { expect(presenter.description).to eq Gitlab::Emoji.emojis[emoji_name]['description'] } + end + + describe '#unicode' do + it { expect(presenter.unicode).to eq Gitlab::Emoji.emojis[emoji_name]['unicode'] } + end + + describe '#unicode_version' do + it { expect(presenter.unicode_version).to eq Gitlab::Emoji.emoji_unicode_version(emoji_name) } + end + + describe '#emoji' do + it { expect(presenter.emoji).to eq Gitlab::Emoji.emojis[emoji_name]['moji'] } + end + + describe 'when presenting an award emoji with an invalid name' do + let(:emoji_name) { 'invalid-name' } + + it 'returns nil for all properties' do + expect(presenter.description).to be_nil + expect(presenter.emoji).to be_nil + expect(presenter.unicode).to be_nil + expect(presenter.unicode_version).to be_nil + end + end +end diff --git a/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb new file mode 100644 index 00000000000..3982125a38a --- /dev/null +++ b/spec/requests/api/graphql/mutations/award_emojis/add_spec.rb @@ -0,0 +1,100 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Adding an AwardEmoji' do + include GraphqlHelpers + + let(:current_user) { create(:user) } + let(:awardable) { create(:note) } + let(:project) { awardable.project } + let(:emoji_name) { 'thumbsup' } + let(:mutation) do + variables = { + awardable_id: GitlabSchema.id_from_object(awardable).to_s, + name: emoji_name + } + + graphql_mutation(:add_award_emoji, variables) + end + + def mutation_response + graphql_mutation_response(:add_award_emoji) + end + + shared_examples 'a mutation that does not create an AwardEmoji' do + it do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { AwardEmoji.count } + end + end + + context 'when the user does not have permission' do + it_behaves_like 'a mutation that does not create an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + end + + context 'when the user has permission' do + before do + project.add_developer(current_user) + end + + context 'when the given awardable is not an Awardable' do + let(:awardable) { create(:label) } + + it_behaves_like 'a mutation that does not create an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Cannot award emoji to this resource'] + end + + context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do + let(:awardable) { create(:system_note) } + + it_behaves_like 'a mutation that does not create an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Cannot award emoji to this resource'] + end + + context 'when the given awardable an Awardable' do + it 'creates an emoji' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { AwardEmoji.count }.by(1) + end + + it 'returns the emoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['awardEmoji']['name']).to eq(emoji_name) + end + + context 'when there were active record validation errors' do + before do + expect_next_instance_of(AwardEmoji) do |award| + expect(award).to receive(:valid?).at_least(:once).and_return(false) + expect(award).to receive_message_chain( + :errors, + :full_messages + ).and_return(['Error 1', 'Error 2']) + end + end + + it_behaves_like 'a mutation that does not create an AwardEmoji' + + it_behaves_like 'a mutation that returns errors in the response', errors: ['Error 1', 'Error 2'] + + it 'returns an empty awardEmoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('awardEmoji') + expect(mutation_response['awardEmoji']).to be_nil + end + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb new file mode 100644 index 00000000000..c78f0c7ca27 --- /dev/null +++ b/spec/requests/api/graphql/mutations/award_emojis/remove_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Removing an AwardEmoji' do + include GraphqlHelpers + + let(:current_user) { create(:user) } + let(:awardable) { create(:note) } + let(:project) { awardable.project } + let(:emoji_name) { 'thumbsup' } + let(:input) { { awardable_id: GitlabSchema.id_from_object(awardable).to_s, name: emoji_name } } + + let(:mutation) do + graphql_mutation(:remove_award_emoji, input) + end + + def mutation_response + graphql_mutation_response(:remove_award_emoji) + end + + def create_award_emoji(user) + create(:award_emoji, name: emoji_name, awardable: awardable, user: user ) + end + + shared_examples 'a mutation that does not destroy an AwardEmoji' do + it do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { AwardEmoji.count } + end + end + + shared_examples 'a mutation that does not authorize the user' do + it_behaves_like 'a mutation that does not destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + end + + context 'when the current_user does not own the award emoji' do + let!(:award_emoji) { create_award_emoji(create(:user)) } + + it_behaves_like 'a mutation that does not authorize the user' + end + + context 'when the current_user owns the award emoji' do + let!(:award_emoji) { create_award_emoji(current_user) } + + context 'when the given awardable is not an Awardable' do + let(:awardable) { create(:label) } + + it_behaves_like 'a mutation that does not destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Cannot award emoji to this resource'] + end + + context 'when the given awardable is an Awardable' do + it 'removes the emoji' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { AwardEmoji.count }.by(-1) + end + + it 'returns no errors' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_errors).to be_nil + end + + it 'returns an empty awardEmoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('awardEmoji') + expect(mutation_response['awardEmoji']).to be_nil + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb new file mode 100644 index 00000000000..31145730f10 --- /dev/null +++ b/spec/requests/api/graphql/mutations/award_emojis/toggle_spec.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Toggling an AwardEmoji' do + include GraphqlHelpers + + let(:current_user) { create(:user) } + let(:awardable) { create(:note) } + let(:project) { awardable.project } + let(:emoji_name) { 'thumbsup' } + let(:mutation) do + variables = { + awardable_id: GitlabSchema.id_from_object(awardable).to_s, + name: emoji_name + } + + graphql_mutation(:toggle_award_emoji, variables) + end + + def mutation_response + graphql_mutation_response(:toggle_award_emoji) + end + + shared_examples 'a mutation that does not create or destroy an AwardEmoji' do + it do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.not_to change { AwardEmoji.count } + end + end + + def create_award_emoji(user) + create(:award_emoji, name: emoji_name, awardable: awardable, user: user ) + end + + context 'when the user has permission' do + before do + project.add_developer(current_user) + end + + context 'when the given awardable is not an Awardable' do + let(:awardable) { create(:label) } + + it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Cannot award emoji to this resource'] + end + + context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do + let(:awardable) { create(:system_note) } + + it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['Cannot award emoji to this resource'] + end + + context 'when the given awardable is an Awardable' do + context 'when no emoji has been awarded by the current_user yet' do + # Create an award emoji for another user. This therefore tests that + # toggling is correctly scoped to the user's emoji only. + let!(:award_emoji) { create_award_emoji(create(:user)) } + + it 'creates an emoji' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { AwardEmoji.count }.by(1) + end + + it 'returns the emoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['awardEmoji']['name']).to eq(emoji_name) + end + + it 'returns toggledOn as true' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['toggledOn']).to eq(true) + end + + context 'when there were active record validation errors' do + before do + expect_next_instance_of(AwardEmoji) do |award| + expect(award).to receive(:valid?).at_least(:once).and_return(false) + expect(award).to receive_message_chain(:errors, :full_messages).and_return(['Error 1', 'Error 2']) + end + end + + it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns errors in the response', errors: ['Error 1', 'Error 2'] + + it 'returns an empty awardEmoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('awardEmoji') + expect(mutation_response['awardEmoji']).to be_nil + end + end + end + + context 'when an emoji has been awarded by the current_user' do + let!(:award_emoji) { create_award_emoji(current_user) } + + it 'removes the emoji' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + end.to change { AwardEmoji.count }.by(-1) + end + + it 'returns no errors' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_errors).to be_nil + end + + it 'returns an empty awardEmoji' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response).to have_key('awardEmoji') + expect(mutation_response['awardEmoji']).to be_nil + end + + it 'returns toggledOn as false' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['toggledOn']).to eq(false) + end + end + end + end + + context 'when the user does not have permission' do + it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' + + it_behaves_like 'a mutation that returns top-level errors', + errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action'] + end +end diff --git a/spec/requests/api/graphql/project/tree/tree_spec.rb b/spec/requests/api/graphql/project/tree/tree_spec.rb index b07aa1e12d3..94128cc21ee 100644 --- a/spec/requests/api/graphql/project/tree/tree_spec.rb +++ b/spec/requests/api/graphql/project/tree/tree_spec.rb @@ -33,6 +33,12 @@ describe 'getting a tree in a project' do expect(graphql_data['project']['repository']['tree']['submodules']['edges']).to eq([]) expect(graphql_data['project']['repository']['tree']['blobs']['edges']).to eq([]) end + + it 'returns null commit' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['project']['repository']['last_commit']).to be_nil + end end context 'when ref does not exist' do @@ -45,6 +51,12 @@ describe 'getting a tree in a project' do expect(graphql_data['project']['repository']['tree']['submodules']['edges']).to eq([]) expect(graphql_data['project']['repository']['tree']['blobs']['edges']).to eq([]) end + + it 'returns null commit' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['project']['repository']['last_commit']).to be_nil + end end context 'when ref and path exist' do @@ -61,6 +73,12 @@ describe 'getting a tree in a project' do expect(graphql_data['project']['repository']['tree']['blobs']['edges'].size).to be > 0 expect(graphql_data['project']['repository']['tree']['submodules']['edges'].size).to be > 0 end + + it 'returns tree latest commit' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['project']['repository']['tree']['lastCommit']).to be_present + end end context 'when current user is nil' do diff --git a/spec/support/api/boards_shared_examples.rb b/spec/support/api/boards_shared_examples.rb index 592962ebf7c..3abb5096a7a 100644 --- a/spec/support/api/boards_shared_examples.rb +++ b/spec/support/api/boards_shared_examples.rb @@ -14,6 +14,16 @@ shared_examples_for 'group and project boards' do |route_definition, ee = false| end end + it 'avoids N+1 queries' do + pat = create(:personal_access_token, user: user) + control = ActiveRecord::QueryRecorder.new { get api(root_url, personal_access_token: pat) } + + create(:milestone, "#{board_parent.class.name.underscore}": board_parent) + create(:board, "#{board_parent.class.name.underscore}": board_parent) + + expect { get api(root_url, personal_access_token: pat) }.not_to exceed_query_limit(control) + end + describe "GET #{route_definition}" do context "when unauthenticated" do it "returns authentication error" do diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index bcf6669f37d..1a09d48f4cd 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -4,10 +4,7 @@ module GraphqlHelpers # makes an underscored string look like a fieldname # "merge_request" => "mergeRequest" def self.fieldnamerize(underscored_field_name) - graphql_field_name = underscored_field_name.to_s.camelize - graphql_field_name[0] = graphql_field_name[0].downcase - - graphql_field_name + underscored_field_name.to_s.camelize(:lower) end # Run a loader's named resolver diff --git a/spec/support/helpers/stub_configuration.rb b/spec/support/helpers/stub_configuration.rb index 0d591f038ce..c372a3f0e49 100644 --- a/spec/support/helpers/stub_configuration.rb +++ b/spec/support/helpers/stub_configuration.rb @@ -95,6 +95,11 @@ module StubConfiguration allow(Gitlab.config.gitlab_shell).to receive_messages(to_settings(messages)) end + def stub_rack_attack_setting(messages) + allow(Gitlab.config.rack_attack).to receive(:git_basic_auth).and_return(messages) + allow(Gitlab.config.rack_attack.git_basic_auth).to receive_messages(to_settings(messages)) + end + private # Modifies stubbed messages to also stub possible predicate versions diff --git a/spec/support/shared_examples/graphql/mutation_shared_examples.rb b/spec/support/shared_examples/graphql/mutation_shared_examples.rb new file mode 100644 index 00000000000..022d41c0bdd --- /dev/null +++ b/spec/support/shared_examples/graphql/mutation_shared_examples.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +# Shared example for expecting top-level errors. +# See https://graphql-ruby.org/mutations/mutation_errors#raising-errors +# +# { errors: [] } +# +# There must be a method or let called `mutation` defined that executes +# the mutation. +RSpec.shared_examples 'a mutation that returns top-level errors' do |errors:| + it do + post_graphql_mutation(mutation, current_user: current_user) + + error_messages = graphql_errors.map { |e| e['message'] } + + expect(error_messages).to eq(errors) + end +end + +# Shared example for expecting schema-level errors. +# See https://graphql-ruby.org/mutations/mutation_errors#errors-as-data +# +# { data: { mutationName: { errors: [] } } } +# +# There must be: +# - a method or let called `mutation` defined that executes the mutation +# - a `mutation_response` method defined that returns the data of the mutation response. +RSpec.shared_examples 'a mutation that returns errors in the response' do |errors:| + it do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['errors']).to eq(errors) + end +end diff --git a/spec/support/shared_examples/services/boards/issues_move_service.rb b/spec/support/shared_examples/services/boards/issues_move_service.rb index 9dbd1d8e867..5359831f8f8 100644 --- a/spec/support/shared_examples/services/boards/issues_move_service.rb +++ b/spec/support/shared_examples/services/boards/issues_move_service.rb @@ -1,8 +1,17 @@ shared_examples 'issues move service' do |group| + shared_examples 'updating timestamps' do + it 'updates updated_at' do + expect {described_class.new(parent, user, params).execute(issue)} + .to change {issue.reload.updated_at} + end + end + context 'when moving an issue between lists' do let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) } let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list2.id } } + it_behaves_like 'updating timestamps' + it 'delegates the label changes to Issues::UpdateService' do service = double(:service) expect(Issues::UpdateService).to receive(:new).and_return(service) @@ -24,6 +33,8 @@ shared_examples 'issues move service' do |group| let(:issue) { create(:labeled_issue, project: project, labels: [bug, development, testing, regression]) } let(:params) { { board_id: board1.id, from_list_id: list2.id, to_list_id: closed.id } } + it_behaves_like 'updating timestamps' + it 'delegates the close proceedings to Issues::CloseService' do expect_any_instance_of(Issues::CloseService).to receive(:execute).with(issue).once @@ -46,6 +57,8 @@ shared_examples 'issues move service' do |group| let(:issue) { create(:labeled_issue, project: project, labels: [bug, development, testing, regression], milestone: milestone) } let(:params) { { board_id: board1.id, from_list_id: list2.id, to_list_id: backlog.id } } + it_behaves_like 'updating timestamps' + it 'keeps labels and milestone' do described_class.new(parent, user, params).execute(issue) issue.reload @@ -59,6 +72,8 @@ shared_examples 'issues move service' do |group| let(:issue) { create(:labeled_issue, :closed, project: project, labels: [bug]) } let(:params) { { board_id: board1.id, from_list_id: closed.id, to_list_id: list2.id } } + it_behaves_like 'updating timestamps' + it 'delegates the re-open proceedings to Issues::ReopenService' do expect_any_instance_of(Issues::ReopenService).to receive(:execute).with(issue).once @@ -75,10 +90,13 @@ shared_examples 'issues move service' do |group| end context 'when moving to same list' do - let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) } - let(:issue1) { create(:labeled_issue, project: project, labels: [bug, development]) } - let(:issue2) { create(:labeled_issue, project: project, labels: [bug, development]) } - let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list1.id } } + let(:assignee) { create(:user) } + let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list1.id } } + let(:issue1) { create(:labeled_issue, project: project, labels: [bug, development]) } + let(:issue2) { create(:labeled_issue, project: project, labels: [bug, development]) } + let(:issue) do + create(:labeled_issue, project: project, labels: [bug, development], assignees: [assignee]) + end it 'returns false' do expect(described_class.new(parent, user, params).execute(issue)).to eq false @@ -90,18 +108,36 @@ shared_examples 'issues move service' do |group| expect(issue.reload.labels).to contain_exactly(bug, development) end - it 'sorts issues' do - [issue, issue1, issue2].each do |issue| - issue.move_to_end && issue.save! - end + it 'keeps issues assignees' do + described_class.new(parent, user, params).execute(issue) + + expect(issue.reload.assignees).to contain_exactly(assignee) + end - params.merge!(move_after_id: issue1.id, move_before_id: issue2.id) + it 'sorts issues' do + reorder_issues(params, issues: [issue, issue1, issue2]) described_class.new(parent, user, params).execute(issue) expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position) end + it 'does not update updated_at' do + reorder_issues(params, issues: [issue, issue1, issue2]) + + updated_at = issue.updated_at + updated_at1 = issue1.updated_at + updated_at2 = issue2.updated_at + + Timecop.travel(1.minute.from_now) do + described_class.new(parent, user, params).execute(issue) + end + + expect(issue.reload.updated_at.change(usec: 0)).to eq updated_at.change(usec: 0) + expect(issue1.reload.updated_at.change(usec: 0)).to eq updated_at1.change(usec: 0) + expect(issue2.reload.updated_at.change(usec: 0)).to eq updated_at2.change(usec: 0) + end + if group context 'when on a group board' do it 'sends the board_group_id parameter' do @@ -114,5 +150,13 @@ shared_examples 'issues move service' do |group| end end end + + def reorder_issues(params, issues: []) + issues.each do |issue| + issue.move_to_end && issue.save! + end + + params.merge!(move_after_id: issues[1].id, move_before_id: issues[2].id) + end end end |