From fa5ca3519ed27787cb302f0c62f791def0834038 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 7 Oct 2021 00:09:20 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- .../customer_relations/contacts/create.rb | 69 +++++++++ app/graphql/types/mutation_type.rb | 1 + app/models/project_setting.rb | 4 - app/policies/group_policy.rb | 3 +- app/serializers/merge_request_metrics_helper.rb | 28 ++++ .../merge_request_poll_cached_widget_entity.rb | 25 +--- .../customer_relations/contacts/base_service.rb | 17 +++ .../customer_relations/contacts/create_service.rb | 41 ++++++ .../organizations/base_service.rb | 2 +- .../organizations/create_service.rb | 4 +- app/views/shared/builds/_tabs.html.haml | 41 +++--- doc/api/graphql/reference/index.md | 25 ++++ doc/user/packages/dependency_proxy/index.md | 2 + .../github_import/representation/diff_note.rb | 12 +- .../diff_notes/suggestion_formatter.rb | 66 +++++++++ qa/qa/resource/fork.rb | 7 + qa/qa/resource/merge_request_from_fork.rb | 13 +- .../merge_merge_request_from_fork_spec.rb | 27 ++-- spec/features/admin/admin_builds_spec.rb | 141 ------------------ spec/features/admin/admin_jobs_spec.rb | 141 ++++++++++++++++++ spec/features/projects/jobs_spec.rb | 8 +- .../customer_relations/contacts/create_spec.rb | 104 +++++++++++++ .../organizations/create_spec.rb | 6 +- .../organizations/update_spec.rb | 10 +- .../importer/diff_notes_importer_spec.rb | 12 +- .../github_import/representation/diff_note_spec.rb | 117 ++++++++++++--- .../diff_notes/suggestion_formatter_spec.rb | 164 +++++++++++++++++++++ .../merge_request_metrics_helper_spec.rb | 69 +++++++++ .../contacts/create_service_spec.rb | 61 ++++++++ .../organizations/create_service_spec.rb | 10 +- .../organizations/update_service_spec.rb | 4 +- 31 files changed, 986 insertions(+), 248 deletions(-) create mode 100644 app/graphql/mutations/customer_relations/contacts/create.rb create mode 100644 app/serializers/merge_request_metrics_helper.rb create mode 100644 app/services/customer_relations/contacts/base_service.rb create mode 100644 app/services/customer_relations/contacts/create_service.rb create mode 100644 lib/gitlab/github_import/representation/diff_notes/suggestion_formatter.rb delete mode 100644 spec/features/admin/admin_builds_spec.rb create mode 100644 spec/features/admin/admin_jobs_spec.rb create mode 100644 spec/graphql/mutations/customer_relations/contacts/create_spec.rb create mode 100644 spec/lib/gitlab/github_import/representation/diff_notes/suggestion_formatter_spec.rb create mode 100644 spec/serializers/merge_request_metrics_helper_spec.rb create mode 100644 spec/services/customer_relations/contacts/create_service_spec.rb diff --git a/app/graphql/mutations/customer_relations/contacts/create.rb b/app/graphql/mutations/customer_relations/contacts/create.rb new file mode 100644 index 00000000000..43f44637042 --- /dev/null +++ b/app/graphql/mutations/customer_relations/contacts/create.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +module Mutations + module CustomerRelations + module Contacts + class Create < BaseMutation + include ResolvesIds + include Gitlab::Graphql::Authorize::AuthorizeResource + + graphql_name 'CustomerRelationsContactCreate' + + field :contact, + Types::CustomerRelations::ContactType, + null: true, + description: 'Contact after the mutation.' + + argument :group_id, ::Types::GlobalIDType[::Group], + required: true, + description: 'Group for the contact.' + + argument :organization_id, ::Types::GlobalIDType[::CustomerRelations::Organization], + required: false, + description: 'Organization for the contact.' + + argument :first_name, GraphQL::Types::String, + required: true, + description: 'First name of the contact.' + + argument :last_name, GraphQL::Types::String, + required: true, + description: 'Last name of the contact.' + + argument :phone, GraphQL::Types::String, + required: false, + description: 'Phone number of the contact.' + + argument :email, GraphQL::Types::String, + required: false, + description: 'Email address of the contact.' + + argument :description, GraphQL::Types::String, + required: false, + description: 'Description or notes for the contact.' + + authorize :admin_contact + + def resolve(args) + group = authorized_find!(id: args[:group_id]) + + raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature disabled' unless Feature.enabled?(:customer_relations, group, default_enabled: :yaml) + + set_organization!(args) + result = ::CustomerRelations::Contacts::CreateService.new(group: group, current_user: current_user, params: args).execute + { contact: result.payload, errors: result.errors } + end + + def find_object(id:) + GitlabSchema.object_from_id(id, expected_type: ::Group) + end + + def set_organization!(args) + return unless args[:organization_id] + + args[:organization_id] = resolve_ids(args[:organization_id], ::Types::GlobalIDType[::CustomerRelations::Organization])[0] + end + end + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index a3be4fcba92..9d4078f873a 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -38,6 +38,7 @@ module Types mount_mutation Mutations::Commits::Create, calls_gitaly: true mount_mutation Mutations::CustomEmoji::Create, feature_flag: :custom_emoji mount_mutation Mutations::CustomEmoji::Destroy, feature_flag: :custom_emoji + mount_mutation Mutations::CustomerRelations::Contacts::Create mount_mutation Mutations::CustomerRelations::Organizations::Create mount_mutation Mutations::CustomerRelations::Organizations::Update mount_mutation Mutations::Discussions::ToggleResolve diff --git a/app/models/project_setting.rb b/app/models/project_setting.rb index b2559636f32..24d892290a6 100644 --- a/app/models/project_setting.rb +++ b/app/models/project_setting.rb @@ -1,10 +1,6 @@ # frozen_string_literal: true class ProjectSetting < ApplicationRecord - include IgnorableColumns - - ignore_column :allow_editing_commit_messages, remove_with: '14.4', remove_after: '2021-09-10' - belongs_to :project, inverse_of: :project_setting enum squash_option: { diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 7abffd2c352..ac8165dfd27 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -134,6 +134,8 @@ class GroupPolicy < BasePolicy enable :create_package enable :create_package_settings enable :developer_access + enable :admin_organization + enable :admin_contact end rule { reporter }.policy do @@ -147,7 +149,6 @@ class GroupPolicy < BasePolicy enable :read_prometheus enable :read_package enable :read_package_settings - enable :admin_organization end rule { maintainer }.policy do diff --git a/app/serializers/merge_request_metrics_helper.rb b/app/serializers/merge_request_metrics_helper.rb new file mode 100644 index 00000000000..fb1769d0aa6 --- /dev/null +++ b/app/serializers/merge_request_metrics_helper.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module MergeRequestMetricsHelper + # There are cases where where metrics object doesn't exist and it needs to be rebuilt. + # TODO: Once https://gitlab.com/gitlab-org/gitlab/-/issues/342508 has been resolved and + # all merge requests have metrics we can remove this helper method. + def build_metrics(merge_request) + # There's no need to query and serialize metrics data for merge requests that are not + # merged or closed. + return unless merge_request.merged? || merge_request.closed? + return merge_request.metrics if merge_request.merged? && merge_request.metrics&.merged_by_id + return merge_request.metrics if merge_request.closed? && merge_request.metrics&.latest_closed_by_id + + build_metrics_from_events(merge_request) + end + + private + + def build_metrics_from_events(merge_request) + closed_event = merge_request.closed_event + merge_event = merge_request.merge_event + + MergeRequest::Metrics.new(latest_closed_at: closed_event&.updated_at, + latest_closed_by: closed_event&.author, + merged_at: merge_event&.updated_at, + merged_by: merge_event&.author) + end +end diff --git a/app/serializers/merge_request_poll_cached_widget_entity.rb b/app/serializers/merge_request_poll_cached_widget_entity.rb index 7fba52cbe17..8b0f3c8eb74 100644 --- a/app/serializers/merge_request_poll_cached_widget_entity.rb +++ b/app/serializers/merge_request_poll_cached_widget_entity.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class MergeRequestPollCachedWidgetEntity < IssuableEntity + include MergeRequestMetricsHelper + expose :auto_merge_enabled expose :state expose :merged_commit_sha @@ -158,29 +160,6 @@ class MergeRequestPollCachedWidgetEntity < IssuableEntity @presenters ||= {} @presenters[merge_request] ||= MergeRequestPresenter.new(merge_request, current_user: current_user) # rubocop: disable CodeReuse/Presenter end - - # Once SchedulePopulateMergeRequestMetricsWithEventsData fully runs, - # we can remove this method and just serialize MergeRequest#metrics - # instead. See https://gitlab.com/gitlab-org/gitlab-foss/issues/41587 - def build_metrics(merge_request) - # There's no need to query and serialize metrics data for merge requests that are not - # merged or closed. - return unless merge_request.merged? || merge_request.closed? - return merge_request.metrics if merge_request.merged? && merge_request.metrics&.merged_by_id - return merge_request.metrics if merge_request.closed? && merge_request.metrics&.latest_closed_by_id - - build_metrics_from_events(merge_request) - end - - def build_metrics_from_events(merge_request) - closed_event = merge_request.closed_event - merge_event = merge_request.merge_event - - MergeRequest::Metrics.new(latest_closed_at: closed_event&.updated_at, - latest_closed_by: closed_event&.author, - merged_at: merge_event&.updated_at, - merged_by: merge_event&.author) - end end MergeRequestPollCachedWidgetEntity.prepend_mod_with('MergeRequestPollCachedWidgetEntity') diff --git a/app/services/customer_relations/contacts/base_service.rb b/app/services/customer_relations/contacts/base_service.rb new file mode 100644 index 00000000000..89f6f2c3f1f --- /dev/null +++ b/app/services/customer_relations/contacts/base_service.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module CustomerRelations + module Contacts + class BaseService < ::BaseGroupService + private + + def allowed? + current_user&.can?(:admin_contact, group) + end + + def error(message) + ServiceResponse.error(message: Array(message)) + end + end + end +end diff --git a/app/services/customer_relations/contacts/create_service.rb b/app/services/customer_relations/contacts/create_service.rb new file mode 100644 index 00000000000..7ff8b731e0d --- /dev/null +++ b/app/services/customer_relations/contacts/create_service.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module CustomerRelations + module Contacts + class CreateService < BaseService + def execute + return error_no_permissions unless allowed? + return error_organization_invalid unless organization_valid? + + contact = Contact.create(params.merge(group_id: group.id)) + + return error_creating(contact) unless contact.persisted? + + ServiceResponse.success(payload: contact) + end + + private + + def organization_valid? + return true unless params[:organization_id] + + organization = Organization.find(params[:organization_id]) + organization.group_id == group.id + rescue ActiveRecord::RecordNotFound + false + end + + def error_organization_invalid + error('The specified organization was not found or does not belong to this group') + end + + def error_no_permissions + error('You have insufficient permissions to create a contact for this group') + end + + def error_creating(contact) + error(contact&.errors&.full_messages || 'Failed to create contact') + end + end + end +end diff --git a/app/services/customer_relations/organizations/base_service.rb b/app/services/customer_relations/organizations/base_service.rb index 63261534b37..8f8480d697c 100644 --- a/app/services/customer_relations/organizations/base_service.rb +++ b/app/services/customer_relations/organizations/base_service.rb @@ -10,7 +10,7 @@ module CustomerRelations end def error(message) - ServiceResponse.error(message: message) + ServiceResponse.error(message: Array(message)) end end end diff --git a/app/services/customer_relations/organizations/create_service.rb b/app/services/customer_relations/organizations/create_service.rb index 9c223796eaf..aad1b7e2ca4 100644 --- a/app/services/customer_relations/organizations/create_service.rb +++ b/app/services/customer_relations/organizations/create_service.rb @@ -7,9 +7,7 @@ module CustomerRelations def execute return error_no_permissions unless allowed? - params[:group_id] = group.id - - organization = Organization.create(params) + organization = Organization.create(params.merge(group_id: group.id)) return error_creating(organization) unless organization.persisted? diff --git a/app/views/shared/builds/_tabs.html.haml b/app/views/shared/builds/_tabs.html.haml index 4973309edf5..498e9cc33ce 100644 --- a/app/views/shared/builds/_tabs.html.haml +++ b/app/views/shared/builds/_tabs.html.haml @@ -1,24 +1,19 @@ -%ul.nav-links.mobile-separator.nav.nav-tabs - %li{ class: active_when(scope.nil?) }> - = link_to build_path_proc.call(nil) do - All - %span.badge.gl-tab-counter-badge.badge-muted.badge-pill.gl-badge.sm.js-totalbuilds-count - = limited_counter_with_delimiter(all_builds) +- count_badge_classes = 'badge badge-muted badge-pill gl-badge gl-tab-counter-badge sm gl-display-none gl-sm-display-inline-flex' - %li{ class: active_when(scope == 'pending') }> - = link_to build_path_proc.call('pending') do - Pending - %span.badge.gl-tab-counter-badge.badge-muted.badge-pill.gl-badge.sm - = limited_counter_with_delimiter(all_builds.pending) - - %li{ class: active_when(scope == 'running') }> - = link_to build_path_proc.call('running') do - Running - %span.badge.gl-tab-counter-badge.badge-muted.badge-pill.gl-badge.sm - = limited_counter_with_delimiter(all_builds.running) - - %li{ class: active_when(scope == 'finished') }> - = link_to build_path_proc.call('finished') do - Finished - %span.badge.gl-tab-counter-badge.badge-muted.badge-pill.gl-badge.sm - = limited_counter_with_delimiter(all_builds.finished) += gl_tabs_nav( {class: 'gl-border-b-0 gl-flex-grow-1', data: { testid: 'jobs-tabs' } } ) do + = gl_tab_link_to build_path_proc.call(nil), { item_active: scope.nil? } do + = _('All') + %span{ class: count_badge_classes } + = limited_counter_with_delimiter(all_builds) + = gl_tab_link_to build_path_proc.call('pending'), { item_active: scope == 'pending' } do + = _('Pending') + %span{ class: count_badge_classes } + = limited_counter_with_delimiter(all_builds.pending) + = gl_tab_link_to build_path_proc.call('running'), { item_active: scope == 'running' } do + = _('Running') + %span{ class: count_badge_classes } + = limited_counter_with_delimiter(all_builds.running) + = gl_tab_link_to build_path_proc.call('finished'), { item_active: scope == 'finished' } do + = _('Finished') + %span{ class: count_badge_classes } + = limited_counter_with_delimiter(all_builds.finished) diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 00f21539848..6c39e4ddd3e 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -1416,6 +1416,31 @@ Input type: `CreateTestCaseInput` | `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | | `testCase` | [`Issue`](#issue) | Test case created. | +### `Mutation.customerRelationsContactCreate` + +Input type: `CustomerRelationsContactCreateInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `description` | [`String`](#string) | Description or notes for the contact. | +| `email` | [`String`](#string) | Email address of the contact. | +| `firstName` | [`String!`](#string) | First name of the contact. | +| `groupId` | [`GroupID!`](#groupid) | Group for the contact. | +| `lastName` | [`String!`](#string) | Last name of the contact. | +| `organizationId` | [`CustomerRelationsOrganizationID`](#customerrelationsorganizationid) | Organization for the contact. | +| `phone` | [`String`](#string) | Phone number of the contact. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `contact` | [`CustomerRelationsContact`](#customerrelationscontact) | Contact after the mutation. | +| `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | + ### `Mutation.customerRelationsOrganizationCreate` Input type: `CustomerRelationsOrganizationCreateInput` diff --git a/doc/user/packages/dependency_proxy/index.md b/doc/user/packages/dependency_proxy/index.md index 1e09559c7fc..04991414e3d 100644 --- a/doc/user/packages/dependency_proxy/index.md +++ b/doc/user/packages/dependency_proxy/index.md @@ -177,6 +177,8 @@ To reclaim disk space used by image blobs that are no longer needed, use the [Dependency Proxy API](../../../api/dependency_proxy.md) to clear the entire cache. +If you clear the cache, the next time a pipeline runs it must pull an image or tag from Docker Hub. + ### Cleanup policies > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/294187) in GitLab Free 14.4. diff --git a/lib/gitlab/github_import/representation/diff_note.rb b/lib/gitlab/github_import/representation/diff_note.rb index 1d6dfbead5f..a3dcd2e380c 100644 --- a/lib/gitlab/github_import/representation/diff_note.rb +++ b/lib/gitlab/github_import/representation/diff_note.rb @@ -40,7 +40,9 @@ module Gitlab note: note.body, created_at: note.created_at, updated_at: note.updated_at, - note_id: note.id + note_id: note.id, + end_line: note.line, + start_line: note.start_line } new(hash) @@ -83,6 +85,14 @@ module Gitlab } end + def note + @note ||= DiffNotes::SuggestionFormatter.formatted_note_for( + note: attributes[:note], + start_line: attributes[:start_line], + end_line: attributes[:end_line] + ) + end + def github_identifiers { note_id: note_id, diff --git a/lib/gitlab/github_import/representation/diff_notes/suggestion_formatter.rb b/lib/gitlab/github_import/representation/diff_notes/suggestion_formatter.rb new file mode 100644 index 00000000000..4e5855ee4cd --- /dev/null +++ b/lib/gitlab/github_import/representation/diff_notes/suggestion_formatter.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +# This class replaces Github markdown suggestion tag with +# a Gitlab suggestion tag. The difference between +# Github's and Gitlab's suggestion tags is that Gitlab +# includes the range of the suggestion in the tag, while Github +# uses other note attributes to position the suggestion. +module Gitlab + module GithubImport + module Representation + module DiffNotes + class SuggestionFormatter + # A github suggestion: + # - the ```suggestion tag must be the first text of the line + # - it might have up to 3 spaces before the ```suggestion tag + # - extra text on the ```suggestion tag line will be ignored + GITHUB_SUGGESTION = /^\ {,3}(?```suggestion\b).*(?\R)/.freeze + + def self.formatted_note_for(...) + new(...).formatted_note + end + + def initialize(note:, start_line: nil, end_line: nil) + @note = note + @start_line = start_line + @end_line = end_line + end + + def formatted_note + if contains_suggestion? + note.gsub( + GITHUB_SUGGESTION, + "\\k:#{suggestion_range}\\k" + ) + else + note + end + end + + private + + attr_reader :note, :start_line, :end_line + + def contains_suggestion? + note.to_s.match?(GITHUB_SUGGESTION) + end + + # Github always saves the comment on the _last_ line of the range. + # Therefore, the diff hunk will always be related to lines before + # the comment itself. + def suggestion_range + "-#{line_count}+0" + end + + def line_count + if start_line.present? + end_line - start_line + else + 0 + end + end + end + end + end + end +end diff --git a/qa/qa/resource/fork.rb b/qa/qa/resource/fork.rb index 106d1d5548a..b3814011f2c 100644 --- a/qa/qa/resource/fork.rb +++ b/qa/qa/resource/fork.rb @@ -14,6 +14,7 @@ module QA resource.add_name_uuid = false resource.name = name resource.path_with_namespace = "#{user.username}/#{name}" + resource.api_client = @api_client end end @@ -69,6 +70,12 @@ module QA populate(:project) end + def remove_via_api! + project.remove_via_api! + upstream.remove_via_api! + user.remove_via_api! unless Specs::Helpers::ContextSelector.dot_com? + end + def api_get_path "/projects/#{CGI.escape(path_with_namespace)}" end diff --git a/qa/qa/resource/merge_request_from_fork.rb b/qa/qa/resource/merge_request_from_fork.rb index 4eebbdf0a52..b0579cf37b8 100644 --- a/qa/qa/resource/merge_request_from_fork.rb +++ b/qa/qa/resource/merge_request_from_fork.rb @@ -6,7 +6,7 @@ module QA attr_accessor :fork_branch attribute :fork do - Fork.fabricate_via_browser_ui! + Fork.fabricate_via_api! end attribute :push do @@ -23,8 +23,15 @@ module QA fork.project.visit! - Page::Project::Show.perform(&:new_merge_request) - Page::MergeRequest::New.perform(&:create_merge_request) + mr_url = Flow::Login.while_signed_in(as: fork.user) do + Page::Project::Show.perform(&:new_merge_request) + Page::MergeRequest::New.perform(&:create_merge_request) + + current_url + end + + Flow::Login.sign_in + visit(mr_url) end def fabricate_via_api! diff --git a/qa/qa/specs/features/browser_ui/3_create/merge_request/merge_merge_request_from_fork_spec.rb b/qa/qa/specs/features/browser_ui/3_create/merge_request/merge_merge_request_from_fork_spec.rb index 4090837d5c9..f2d4fc6e677 100644 --- a/qa/qa/specs/features/browser_ui/3_create/merge_request/merge_merge_request_from_fork_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/merge_request/merge_merge_request_from_fork_spec.rb @@ -1,22 +1,29 @@ # frozen_string_literal: true module QA - RSpec.describe 'Create', quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/332588', type: :investigating } do + RSpec.describe 'Create' do describe 'Merge request creation from fork' do - # TODO: Please add this back to :smoke suite as soon as https://gitlab.com/gitlab-org/gitlab/-/issues/332588 is addressed - it 'can merge feature branch fork to mainline', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/1305' do - merge_request = Resource::MergeRequestFromFork.fabricate_via_browser_ui! do |merge_request| + let(:merge_request) do + Resource::MergeRequestFromFork.fabricate_via_browser_ui! do |merge_request| merge_request.fork_branch = 'feature-branch' end + end + + before do + Flow::Login.sign_in + end - Flow::Login.while_signed_in do - merge_request.visit! + after do + merge_request.fork.remove_via_api! + end + + it 'can merge feature branch fork to mainline', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/1305' do + merge_request.visit! - Page::MergeRequest::Show.perform do |merge_request| - merge_request.merge! + Page::MergeRequest::Show.perform do |merge_request| + merge_request.merge! - expect(merge_request).to have_content('The changes were merged') - end + expect(merge_request).to be_merged end end end diff --git a/spec/features/admin/admin_builds_spec.rb b/spec/features/admin/admin_builds_spec.rb deleted file mode 100644 index 42827dd5b49..00000000000 --- a/spec/features/admin/admin_builds_spec.rb +++ /dev/null @@ -1,141 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Admin Builds' do - before do - admin = create(:admin) - sign_in(admin) - gitlab_enable_admin_mode_sign_in(admin) - end - - describe 'GET /admin/builds' do - let(:pipeline) { create(:ci_pipeline) } - - context 'All tab' do - context 'when have jobs' do - it 'shows all jobs', :js do - create(:ci_build, pipeline: pipeline, status: :pending) - create(:ci_build, pipeline: pipeline, status: :running) - create(:ci_build, pipeline: pipeline, status: :success) - create(:ci_build, pipeline: pipeline, status: :failed) - - visit admin_jobs_path - - expect(page).to have_selector('.nav-links li.active', text: 'All') - expect(page).to have_selector('.row-content-block', text: 'All jobs') - expect(page.all('.build-link').size).to eq(4) - expect(page).to have_button 'Stop all jobs' - - click_button 'Stop all jobs' - expect(page).to have_button 'Stop jobs' - expect(page).to have_content 'Stop all jobs?' - end - end - - context 'when have no jobs' do - it 'shows a message' do - visit admin_jobs_path - - expect(page).to have_selector('.nav-links li.active', text: 'All') - expect(page).to have_content 'No jobs to show' - expect(page).not_to have_button 'Stop all jobs' - end - end - end - - context 'Pending tab' do - context 'when have pending jobs' do - it 'shows pending jobs' do - build1 = create(:ci_build, pipeline: pipeline, status: :pending) - build2 = create(:ci_build, pipeline: pipeline, status: :running) - build3 = create(:ci_build, pipeline: pipeline, status: :success) - build4 = create(:ci_build, pipeline: pipeline, status: :failed) - - visit admin_jobs_path(scope: :pending) - - expect(page).to have_selector('.nav-links li.active', text: 'Pending') - expect(page.find('.build-link')).to have_content(build1.id) - expect(page.find('.build-link')).not_to have_content(build2.id) - expect(page.find('.build-link')).not_to have_content(build3.id) - expect(page.find('.build-link')).not_to have_content(build4.id) - expect(page).to have_button 'Stop all jobs' - end - end - - context 'when have no jobs pending' do - it 'shows a message' do - create(:ci_build, pipeline: pipeline, status: :success) - - visit admin_jobs_path(scope: :pending) - - expect(page).to have_selector('.nav-links li.active', text: 'Pending') - expect(page).to have_content 'No jobs to show' - expect(page).not_to have_button 'Stop all jobs' - end - end - end - - context 'Running tab' do - context 'when have running jobs' do - it 'shows running jobs' do - build1 = create(:ci_build, pipeline: pipeline, status: :running) - build2 = create(:ci_build, pipeline: pipeline, status: :success) - build3 = create(:ci_build, pipeline: pipeline, status: :failed) - build4 = create(:ci_build, pipeline: pipeline, status: :pending) - - visit admin_jobs_path(scope: :running) - - expect(page).to have_selector('.nav-links li.active', text: 'Running') - expect(page.find('.build-link')).to have_content(build1.id) - expect(page.find('.build-link')).not_to have_content(build2.id) - expect(page.find('.build-link')).not_to have_content(build3.id) - expect(page.find('.build-link')).not_to have_content(build4.id) - expect(page).to have_button 'Stop all jobs' - end - end - - context 'when have no jobs running' do - it 'shows a message' do - create(:ci_build, pipeline: pipeline, status: :success) - - visit admin_jobs_path(scope: :running) - - expect(page).to have_selector('.nav-links li.active', text: 'Running') - expect(page).to have_content 'No jobs to show' - expect(page).not_to have_button 'Stop all jobs' - end - end - end - - context 'Finished tab' do - context 'when have finished jobs' do - it 'shows finished jobs' do - build1 = create(:ci_build, pipeline: pipeline, status: :pending) - build2 = create(:ci_build, pipeline: pipeline, status: :running) - build3 = create(:ci_build, pipeline: pipeline, status: :success) - - visit admin_jobs_path(scope: :finished) - - expect(page).to have_selector('.nav-links li.active', text: 'Finished') - expect(page.find('.build-link')).not_to have_content(build1.id) - expect(page.find('.build-link')).not_to have_content(build2.id) - expect(page.find('.build-link')).to have_content(build3.id) - expect(page).to have_button 'Stop all jobs' - end - end - - context 'when have no jobs finished' do - it 'shows a message' do - create(:ci_build, pipeline: pipeline, status: :running) - - visit admin_jobs_path(scope: :finished) - - expect(page).to have_selector('.nav-links li.active', text: 'Finished') - expect(page).to have_content 'No jobs to show' - expect(page).to have_button 'Stop all jobs' - end - end - end - end -end diff --git a/spec/features/admin/admin_jobs_spec.rb b/spec/features/admin/admin_jobs_spec.rb new file mode 100644 index 00000000000..36822f89c12 --- /dev/null +++ b/spec/features/admin/admin_jobs_spec.rb @@ -0,0 +1,141 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Admin Jobs' do + before do + admin = create(:admin) + sign_in(admin) + gitlab_enable_admin_mode_sign_in(admin) + end + + describe 'GET /admin/jobs' do + let(:pipeline) { create(:ci_pipeline) } + + context 'All tab' do + context 'when have jobs' do + it 'shows all jobs', :js do + create(:ci_build, pipeline: pipeline, status: :pending) + create(:ci_build, pipeline: pipeline, status: :running) + create(:ci_build, pipeline: pipeline, status: :success) + create(:ci_build, pipeline: pipeline, status: :failed) + + visit admin_jobs_path + + expect(page).to have_selector('[data-testid="jobs-tabs"] a.active', text: 'All') + expect(page).to have_selector('.row-content-block', text: 'All jobs') + expect(page.all('.build-link').size).to eq(4) + expect(page).to have_button 'Stop all jobs' + + click_button 'Stop all jobs' + expect(page).to have_button 'Stop jobs' + expect(page).to have_content 'Stop all jobs?' + end + end + + context 'when have no jobs' do + it 'shows a message' do + visit admin_jobs_path + + expect(page).to have_selector('[data-testid="jobs-tabs"] a.active', text: 'All') + expect(page).to have_content 'No jobs to show' + expect(page).not_to have_button 'Stop all jobs' + end + end + end + + context 'Pending tab' do + context 'when have pending jobs' do + it 'shows pending jobs' do + build1 = create(:ci_build, pipeline: pipeline, status: :pending) + build2 = create(:ci_build, pipeline: pipeline, status: :running) + build3 = create(:ci_build, pipeline: pipeline, status: :success) + build4 = create(:ci_build, pipeline: pipeline, status: :failed) + + visit admin_jobs_path(scope: :pending) + + expect(page).to have_selector('[data-testid="jobs-tabs"] a.active', text: 'Pending') + expect(page.find('.build-link')).to have_content(build1.id) + expect(page.find('.build-link')).not_to have_content(build2.id) + expect(page.find('.build-link')).not_to have_content(build3.id) + expect(page.find('.build-link')).not_to have_content(build4.id) + expect(page).to have_button 'Stop all jobs' + end + end + + context 'when have no jobs pending' do + it 'shows a message' do + create(:ci_build, pipeline: pipeline, status: :success) + + visit admin_jobs_path(scope: :pending) + + expect(page).to have_selector('[data-testid="jobs-tabs"] a.active', text: 'Pending') + expect(page).to have_content 'No jobs to show' + expect(page).not_to have_button 'Stop all jobs' + end + end + end + + context 'Running tab' do + context 'when have running jobs' do + it 'shows running jobs' do + build1 = create(:ci_build, pipeline: pipeline, status: :running) + build2 = create(:ci_build, pipeline: pipeline, status: :success) + build3 = create(:ci_build, pipeline: pipeline, status: :failed) + build4 = create(:ci_build, pipeline: pipeline, status: :pending) + + visit admin_jobs_path(scope: :running) + + expect(page).to have_selector('[data-testid="jobs-tabs"] a.active', text: 'Running') + expect(page.find('.build-link')).to have_content(build1.id) + expect(page.find('.build-link')).not_to have_content(build2.id) + expect(page.find('.build-link')).not_to have_content(build3.id) + expect(page.find('.build-link')).not_to have_content(build4.id) + expect(page).to have_button 'Stop all jobs' + end + end + + context 'when have no jobs running' do + it 'shows a message' do + create(:ci_build, pipeline: pipeline, status: :success) + + visit admin_jobs_path(scope: :running) + + expect(page).to have_selector('[data-testid="jobs-tabs"] a.active', text: 'Running') + expect(page).to have_content 'No jobs to show' + expect(page).not_to have_button 'Stop all jobs' + end + end + end + + context 'Finished tab' do + context 'when have finished jobs' do + it 'shows finished jobs' do + build1 = create(:ci_build, pipeline: pipeline, status: :pending) + build2 = create(:ci_build, pipeline: pipeline, status: :running) + build3 = create(:ci_build, pipeline: pipeline, status: :success) + + visit admin_jobs_path(scope: :finished) + + expect(page).to have_selector('[data-testid="jobs-tabs"] a.active', text: 'Finished') + expect(page.find('.build-link')).not_to have_content(build1.id) + expect(page.find('.build-link')).not_to have_content(build2.id) + expect(page.find('.build-link')).to have_content(build3.id) + expect(page).to have_button 'Stop all jobs' + end + end + + context 'when have no jobs finished' do + it 'shows a message' do + create(:ci_build, pipeline: pipeline, status: :running) + + visit admin_jobs_path(scope: :finished) + + expect(page).to have_selector('[data-testid="jobs-tabs"] a.active', text: 'Finished') + expect(page).to have_content 'No jobs to show' + expect(page).to have_button 'Stop all jobs' + end + end + end + end +end diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index a1416f3f563..7ccd5c51493 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -46,7 +46,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do end it "shows Pending tab jobs" do - expect(page).to have_selector('.nav-links li.active', text: 'Pending') + expect(page).to have_selector('[data-testid="jobs-tabs"] a.active', text: 'Pending') expect(page).to have_content job.short_sha expect(page).to have_content job.ref expect(page).to have_content job.name @@ -60,7 +60,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do end it "shows Running tab jobs" do - expect(page).to have_selector('.nav-links li.active', text: 'Running') + expect(page).to have_selector('[data-testid="jobs-tabs"] a.active', text: 'Running') expect(page).to have_content job.short_sha expect(page).to have_content job.ref expect(page).to have_content job.name @@ -74,7 +74,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do end it "shows Finished tab jobs" do - expect(page).to have_selector('.nav-links li.active', text: 'Finished') + expect(page).to have_selector('[data-testid="jobs-tabs"] a.active', text: 'Finished') expect(page).to have_content('Use jobs to automate your tasks') end end @@ -86,7 +86,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do end it "shows All tab jobs" do - expect(page).to have_selector('.nav-links li.active', text: 'All') + expect(page).to have_selector('[data-testid="jobs-tabs"] a.active', text: 'All') expect(page).to have_content job.short_sha expect(page).to have_content job.ref expect(page).to have_content job.name diff --git a/spec/graphql/mutations/customer_relations/contacts/create_spec.rb b/spec/graphql/mutations/customer_relations/contacts/create_spec.rb new file mode 100644 index 00000000000..d6215aebf66 --- /dev/null +++ b/spec/graphql/mutations/customer_relations/contacts/create_spec.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::CustomerRelations::Contacts::Create do + let_it_be(:user) { create(:user) } + let_it_be(:not_found_or_does_not_belong) { 'The specified organization was not found or does not belong to this group' } + + let(:valid_params) do + attributes_for(:contact, + group: group, + description: 'Managing Director' + ) + end + + describe '#resolve' do + subject(:resolve_mutation) do + described_class.new(object: nil, context: { current_user: user }, field: nil).resolve( + **valid_params, + group_id: group.to_global_id + ) + end + + context 'when the user does not have permission' do + let_it_be(:group) { create(:group) } + + before do + group.add_reporter(user) + end + + it 'raises an error' do + expect { resolve_mutation }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + .with_message("The resource that you are attempting to access does not exist or you don't have permission to perform this action") + end + end + + context 'when the user has permission' do + let_it_be(:group) { create(:group) } + + before_all do + group.add_developer(user) + end + + context 'when the feature is disabled' do + before do + stub_feature_flags(customer_relations: false) + end + + it 'raises an error' do + expect { resolve_mutation }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + .with_message('Feature disabled') + end + end + + context 'when the params are invalid' do + it 'returns the validation error' do + valid_params[:first_name] = nil + + expect(resolve_mutation[:errors]).to match_array(["First name can't be blank"]) + end + end + + context 'when attaching to an organization' do + context 'when all ok' do + before do + organization = create(:organization, group: group) + valid_params[:organization_id] = organization.to_global_id + end + + it 'creates contact with correct values' do + expect(resolve_mutation[:contact].organization).to be_present + end + end + + context 'when organization_id is invalid' do + before do + valid_params[:organization_id] = "gid://gitlab/CustomerRelations::Organization/#{non_existing_record_id}" + end + + it 'returns the relevant error' do + expect(resolve_mutation[:errors]).to match_array([not_found_or_does_not_belong]) + end + end + + context 'when organzation belongs to a different group' do + before do + organization = create(:organization) + valid_params[:organization_id] = organization.to_global_id + end + + it 'returns the relevant error' do + expect(resolve_mutation[:errors]).to match_array([not_found_or_does_not_belong]) + end + end + end + + it 'creates contact with correct values' do + expect(resolve_mutation[:contact]).to have_attributes(valid_params) + end + end + end + + specify { expect(described_class).to require_graphql_authorizations(:admin_contact) } +end diff --git a/spec/graphql/mutations/customer_relations/organizations/create_spec.rb b/spec/graphql/mutations/customer_relations/organizations/create_spec.rb index ab430b9240b..055824b657f 100644 --- a/spec/graphql/mutations/customer_relations/organizations/create_spec.rb +++ b/spec/graphql/mutations/customer_relations/organizations/create_spec.rb @@ -26,11 +26,12 @@ RSpec.describe Mutations::CustomerRelations::Organizations::Create do let_it_be(:group) { create(:group) } before do - group.add_guest(user) + group.add_reporter(user) end it 'raises an error' do expect { resolve_mutation }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + .with_message("The resource that you are attempting to access does not exist or you don't have permission to perform this action") end end @@ -38,7 +39,7 @@ RSpec.describe Mutations::CustomerRelations::Organizations::Create do let_it_be(:group) { create(:group) } before_all do - group.add_reporter(user) + group.add_developer(user) end context 'when the feature is disabled' do @@ -48,6 +49,7 @@ RSpec.describe Mutations::CustomerRelations::Organizations::Create do it 'raises an error' do expect { resolve_mutation }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + .with_message('Feature disabled') end end diff --git a/spec/graphql/mutations/customer_relations/organizations/update_spec.rb b/spec/graphql/mutations/customer_relations/organizations/update_spec.rb index f5aa6c00301..834e592e317 100644 --- a/spec/graphql/mutations/customer_relations/organizations/update_spec.rb +++ b/spec/graphql/mutations/customer_relations/organizations/update_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Mutations::CustomerRelations::Organizations::Update do let_it_be(:name) { 'GitLab' } let_it_be(:default_rate) { 1000.to_f } let_it_be(:description) { 'VIP' } + let_it_be(:does_not_exist_or_no_permission) { "The resource that you are attempting to access does not exist or you don't have permission to perform this action" } let(:organization) { create(:organization, group: group) } let(:attributes) do @@ -29,11 +30,12 @@ RSpec.describe Mutations::CustomerRelations::Organizations::Update do let_it_be(:group) { create(:group) } before do - group.add_guest(user) + group.add_reporter(user) end it 'raises an error' do expect { resolve_mutation }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + .with_message(does_not_exist_or_no_permission) end end @@ -41,9 +43,10 @@ RSpec.describe Mutations::CustomerRelations::Organizations::Update do let_it_be(:group) { create(:group) } it 'raises an error' do - attributes[:id] = 'gid://gitlab/CustomerRelations::Organization/999' + attributes[:id] = "gid://gitlab/CustomerRelations::Organization/#{non_existing_record_id}" expect { resolve_mutation }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + .with_message(does_not_exist_or_no_permission) end end @@ -51,7 +54,7 @@ RSpec.describe Mutations::CustomerRelations::Organizations::Update do let_it_be(:group) { create(:group) } before_all do - group.add_reporter(user) + group.add_developer(user) end it 'updates the organization with correct values' do @@ -65,6 +68,7 @@ RSpec.describe Mutations::CustomerRelations::Organizations::Update do it 'raises an error' do expect { resolve_mutation }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + .with_message('Feature disabled') end end end diff --git a/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb b/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb index 46b9959ff64..be4fc3cbf16 100644 --- a/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb @@ -15,10 +15,18 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNotesImporter do original_commit_id: 'original123abc', diff_hunk: "@@ -1 +1 @@\n-Hello\n+Hello world", user: double(:user, id: 4, login: 'alice'), - body: 'Hello world', created_at: Time.zone.now, updated_at: Time.zone.now, - id: 1 + line: 23, + start_line: nil, + id: 1, + body: <<~BODY + Hello World + + ```suggestion + sug1 + ``` + BODY ) end diff --git a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb index 91479d23f43..81722c0eba7 100644 --- a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb +++ b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb @@ -73,6 +73,8 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNote do body: 'Hello world', created_at: created_at, updated_at: updated_at, + line: 23, + start_line: nil, id: 1 ) end @@ -90,47 +92,70 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNote do expect(note.author).to be_nil end - end - describe '.from_json_hash' do - it_behaves_like 'a DiffNote' do - let(:hash) do - { - 'noteable_type' => 'MergeRequest', - 'noteable_id' => 42, - 'file_path' => 'README.md', - 'commit_id' => '123abc', - 'original_commit_id' => 'original123abc', - 'diff_hunk' => hunk, - 'author' => { 'id' => 4, 'login' => 'alice' }, - 'note' => 'Hello world', - 'created_at' => created_at.to_s, - 'updated_at' => updated_at.to_s, - 'note_id' => 1 - } - end + it 'formats a suggestion in the note body' do + allow(response) + .to receive(:body) + .and_return <<~BODY + ```suggestion + Hello World + ``` + BODY - let(:note) { described_class.from_json_hash(hash) } + note = described_class.from_api_response(response) + + expect(note.note).to eq <<~BODY + ```suggestion:-0+0 + Hello World + ``` + BODY end + end - it 'does not convert the author if it was not specified' do - hash = { + describe '.from_json_hash' do + let(:hash) do + { 'noteable_type' => 'MergeRequest', 'noteable_id' => 42, 'file_path' => 'README.md', 'commit_id' => '123abc', 'original_commit_id' => 'original123abc', 'diff_hunk' => hunk, + 'author' => { 'id' => 4, 'login' => 'alice' }, 'note' => 'Hello world', 'created_at' => created_at.to_s, 'updated_at' => updated_at.to_s, 'note_id' => 1 } + end + + it_behaves_like 'a DiffNote' do + let(:note) { described_class.from_json_hash(hash) } + end + + it 'does not convert the author if it was not specified' do + hash.delete('author') note = described_class.from_json_hash(hash) expect(note.author).to be_nil end + + it 'formats a suggestion in the note body' do + hash['note'] = <<~BODY + ```suggestion + Hello World + ``` + BODY + + note = described_class.from_json_hash(hash) + + expect(note.note).to eq <<~BODY + ```suggestion:-0+0 + Hello World + ``` + BODY + end end describe '#line_code' do @@ -181,4 +206,54 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNote do expect(note.github_identifiers).to eq(github_identifiers) end end + + describe '#note' do + it 'returns the given note' do + hash = { + 'note': 'simple text' + } + + note = described_class.new(hash) + + expect(note.note).to eq 'simple text' + end + + it 'returns the suggestion formatted in the note' do + hash = { + 'note': <<~BODY + ```suggestion + Hello World + ``` + BODY + } + + note = described_class.new(hash) + + expect(note.note).to eq <<~BODY + ```suggestion:-0+0 + Hello World + ``` + BODY + end + + it 'returns the multi-line suggestion formatted in the note' do + hash = { + 'start_line': 20, + 'end_line': 23, + 'note': <<~BODY + ```suggestion + Hello World + ``` + BODY + } + + note = described_class.new(hash) + + expect(note.note).to eq <<~BODY + ```suggestion:-3+0 + Hello World + ``` + BODY + end + end end diff --git a/spec/lib/gitlab/github_import/representation/diff_notes/suggestion_formatter_spec.rb b/spec/lib/gitlab/github_import/representation/diff_notes/suggestion_formatter_spec.rb new file mode 100644 index 00000000000..2ffd5f50d3b --- /dev/null +++ b/spec/lib/gitlab/github_import/representation/diff_notes/suggestion_formatter_spec.rb @@ -0,0 +1,164 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Representation::DiffNotes::SuggestionFormatter do + it 'does nothing when there is any text before the suggestion tag' do + note = <<~BODY + looks like```suggestion but it isn't + ``` + BODY + + expect(described_class.formatted_note_for(note: note)).to eq(note) + end + + it 'handles nil value for note' do + note = nil + + expect(described_class.formatted_note_for(note: note)).to eq(note) + end + + it 'does not allow over 3 leading spaces for valid suggestion' do + note = <<~BODY + Single-line suggestion + ```suggestion + sug1 + ``` + BODY + + expect(described_class.formatted_note_for(note: note)).to eq(note) + end + + it 'allows up to 3 leading spaces' do + note = <<~BODY + Single-line suggestion + ```suggestion + sug1 + ``` + BODY + + expected = <<~BODY + Single-line suggestion + ```suggestion:-0+0 + sug1 + ``` + BODY + + expect(described_class.formatted_note_for(note: note)).to eq(expected) + end + + it 'does nothing when there is any text without space after the suggestion tag' do + note = <<~BODY + ```suggestionbut it isn't + ``` + BODY + + expect(described_class.formatted_note_for(note: note)).to eq(note) + end + + it 'formats single-line suggestions' do + note = <<~BODY + Single-line suggestion + ```suggestion + sug1 + ``` + BODY + + expected = <<~BODY + Single-line suggestion + ```suggestion:-0+0 + sug1 + ``` + BODY + + expect(described_class.formatted_note_for(note: note)).to eq(expected) + end + + it 'ignores text after suggestion tag on the same line' do + note = <<~BODY + looks like + ```suggestion text to be ignored + suggestion + ``` + BODY + + expected = <<~BODY + looks like + ```suggestion:-0+0 + suggestion + ``` + BODY + + expect(described_class.formatted_note_for(note: note)).to eq(expected) + end + + it 'formats multiple single-line suggestions' do + note = <<~BODY + Single-line suggestion + ```suggestion + sug1 + ``` + OR + ```suggestion + sug2 + ``` + BODY + + expected = <<~BODY + Single-line suggestion + ```suggestion:-0+0 + sug1 + ``` + OR + ```suggestion:-0+0 + sug2 + ``` + BODY + + expect(described_class.formatted_note_for(note: note)).to eq(expected) + end + + it 'formats multi-line suggestions' do + note = <<~BODY + Multi-line suggestion + ```suggestion + sug1 + ``` + BODY + + expected = <<~BODY + Multi-line suggestion + ```suggestion:-2+0 + sug1 + ``` + BODY + + expect(described_class.formatted_note_for(note: note, start_line: 6, end_line: 8)).to eq(expected) + end + + it 'formats multiple multi-line suggestions' do + note = <<~BODY + Multi-line suggestion + ```suggestion + sug1 + ``` + OR + ```suggestion + sug2 + ``` + BODY + + expected = <<~BODY + Multi-line suggestion + ```suggestion:-2+0 + sug1 + ``` + OR + ```suggestion:-2+0 + sug2 + ``` + BODY + + expect(described_class.formatted_note_for(note: note, start_line: 6, end_line: 8)).to eq(expected) + end +end diff --git a/spec/serializers/merge_request_metrics_helper_spec.rb b/spec/serializers/merge_request_metrics_helper_spec.rb new file mode 100644 index 00000000000..8f683df1faa --- /dev/null +++ b/spec/serializers/merge_request_metrics_helper_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequestMetricsHelper do + let_it_be(:user) { create(:user) } + + let(:merge_request) { create(:merge_request) } + let(:helper) { Class.new.include(described_class).new } + + describe '#build_metrics' do + subject do + helper.build_metrics(merge_request) + end + + shared_examples 'does not rebuild the metrics' do + it 'does not call the merge request metrics class' do + expect(MergeRequest::Metrics).not_to receive(:new) + + subject + end + + it 'returns the metrics for the given merge request' do + expect(subject).to be_kind_of(MergeRequest::Metrics) + expect(subject[:merge_request_id]).to eq(merge_request.id) + end + end + + context 'when closed and metrics exists' do + before do + merge_request.close! + merge_request.metrics.update!(latest_closed_by: user) + end + + include_examples 'does not rebuild the metrics' + end + + context 'when merged and metrics exists' do + before do + merge_request.mark_as_merged! + merge_request.metrics.update!(merged_by: user) + end + + include_examples 'does not rebuild the metrics' + end + + context 'when merged and metrics do not exists' do + before do + merge_request.mark_as_merged! + merge_request.metrics.destroy! + merge_request.reload + end + + it 'rebuilds the merge request metrics' do + closed_event = merge_request.closed_event + merge_event = merge_request.merge_event + + expect(MergeRequest::Metrics).to receive(:new) + .with(latest_closed_at: closed_event&.updated_at, + latest_closed_by: closed_event&.author, + merged_at: merge_event&.updated_at, + merged_by: merge_event&.author) + .and_call_original + + subject + end + end + end +end diff --git a/spec/services/customer_relations/contacts/create_service_spec.rb b/spec/services/customer_relations/contacts/create_service_spec.rb new file mode 100644 index 00000000000..71eb447055e --- /dev/null +++ b/spec/services/customer_relations/contacts/create_service_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe CustomerRelations::Contacts::CreateService do + describe '#execute' do + let_it_be(:user) { create(:user) } + let_it_be(:not_found_or_does_not_belong) { 'The specified organization was not found or does not belong to this group' } + + let(:params) { attributes_for(:contact, group: group) } + + subject(:response) { described_class.new(group: group, current_user: user, params: params).execute } + + context 'when user does not have permission' do + let_it_be(:group) { create(:group) } + + before_all do + group.add_reporter(user) + end + + it 'returns an error' do + expect(response).to be_error + expect(response.message).to match_array(['You have insufficient permissions to create a contact for this group']) + end + end + + context 'when user has permission' do + let_it_be(:group) { create(:group) } + + before_all do + group.add_developer(user) + end + + it 'creates a contact' do + expect(response).to be_success + end + + it 'returns an error when the contact is not persisted' do + params[:last_name] = nil + + expect(response).to be_error + expect(response.message).to match_array(["Last name can't be blank"]) + end + + it 'returns an error when the organization_id is invalid' do + params[:organization_id] = non_existing_record_id + + expect(response).to be_error + expect(response.message).to match_array([not_found_or_does_not_belong]) + end + + it 'returns an error when the organization belongs to a different group' do + organization = create(:organization) + params[:organization_id] = organization.id + + expect(response).to be_error + expect(response.message).to match_array([not_found_or_does_not_belong]) + end + end + end +end diff --git a/spec/services/customer_relations/organizations/create_service_spec.rb b/spec/services/customer_relations/organizations/create_service_spec.rb index b4764f6b97a..d8985d8d90b 100644 --- a/spec/services/customer_relations/organizations/create_service_spec.rb +++ b/spec/services/customer_relations/organizations/create_service_spec.rb @@ -12,22 +12,24 @@ RSpec.describe CustomerRelations::Organizations::CreateService do subject(:response) { described_class.new(group: group, current_user: user, params: params).execute } it 'creates an organization' do - group.add_reporter(user) + group.add_developer(user) expect(response).to be_success end it 'returns an error when user does not have permission' do + group.add_reporter(user) + expect(response).to be_error - expect(response.message).to eq('You have insufficient permissions to create an organization for this group') + expect(response.message).to match_array(['You have insufficient permissions to create an organization for this group']) end it 'returns an error when the organization is not persisted' do - group.add_reporter(user) + group.add_developer(user) params[:name] = nil expect(response).to be_error - expect(response.message).to eq(["Name can't be blank"]) + expect(response.message).to match_array(["Name can't be blank"]) end end end diff --git a/spec/services/customer_relations/organizations/update_service_spec.rb b/spec/services/customer_relations/organizations/update_service_spec.rb index eb253540863..bc40cb3e8e7 100644 --- a/spec/services/customer_relations/organizations/update_service_spec.rb +++ b/spec/services/customer_relations/organizations/update_service_spec.rb @@ -19,7 +19,7 @@ RSpec.describe CustomerRelations::Organizations::UpdateService do response = update expect(response).to be_error - expect(response.message).to eq('You have insufficient permissions to update an organization for this group') + expect(response.message).to eq(['You have insufficient permissions to update an organization for this group']) end end @@ -27,7 +27,7 @@ RSpec.describe CustomerRelations::Organizations::UpdateService do let_it_be(:group) { create(:group) } before_all do - group.add_reporter(user) + group.add_developer(user) end context 'when name is changed' do -- cgit v1.2.3