summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2022-12-29 18:09:53 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2022-12-29 18:09:53 +0000
commit2dcdb22ddf7f36f4f9581737bc4e639655e9f178 (patch)
treeeb90d2340f7da8c9803d51025bb1ee8498887c37
parentdbdfb3e36dd5ed7b66a029cdd3202d6d7da5ae55 (diff)
downloadgitlab-ce-2dcdb22ddf7f36f4f9581737bc4e639655e9f178.tar.gz
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--app/controllers/application_controller.rb1
-rw-r--r--app/controllers/concerns/observability/content_security_policy.rb10
-rw-r--r--app/controllers/groups/observability_controller.rb4
-rw-r--r--app/controllers/projects/merge_requests/diffs_controller.rb16
-rw-r--r--app/serializers/diffs_entity.rb2
-rw-r--r--app/serializers/diffs_metadata_entity.rb2
-rw-r--r--app/serializers/paginated_diff_entity.rb2
-rw-r--r--config/feature_flags/development/display_merge_conflicts_in_diff.yml8
-rw-r--r--db/post_migrate/20221228210616_add_index_on_ci_runners_on_runner_type_and_id.rb18
-rw-r--r--db/schema_migrations/202212282106161
-rw-r--r--db/structure.sql2
-rw-r--r--doc/user/project/merge_requests/changes.md1
-rw-r--r--lib/gitlab/observability.rb4
-rw-r--r--lib/sidebars/groups/menus/observability_menu.rb2
-rw-r--r--spec/controllers/projects/merge_requests/diffs_controller_spec.rb45
-rw-r--r--spec/finders/ci/runners_finder_spec.rb2
-rw-r--r--spec/lib/gitlab/observability_spec.rb37
-rw-r--r--spec/lib/sidebars/groups/menus/observability_menu_spec.rb10
-rw-r--r--spec/requests/projects/issues_controller_spec.rb25
-rw-r--r--spec/requests/projects/merge_requests/context_commit_diffs_spec.rb1
-rw-r--r--spec/requests/projects/merge_requests/creations_spec.rb15
-rw-r--r--spec/requests/projects/merge_requests/diffs_spec.rb12
-rw-r--r--spec/requests/projects/merge_requests_controller_spec.rb14
-rw-r--r--spec/serializers/diffs_entity_spec.rb7
-rw-r--r--spec/serializers/diffs_metadata_entity_spec.rb7
-rw-r--r--spec/serializers/paginated_diff_entity_spec.rb7
-rw-r--r--spec/support/shared_examples/observability/csp_shared_examples.rb47
27 files changed, 175 insertions, 127 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index e64d3110c3a..36aae42e21f 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -24,6 +24,7 @@ class ApplicationController < ActionController::Base
include ::Gitlab::EndpointAttributes
include FlocOptOut
include CheckRateLimit
+ extend ContentSecurityPolicyPatch
before_action :limit_session_time, if: -> { !current_user }
before_action :authenticate_user!, except: [:route_not_found]
diff --git a/app/controllers/concerns/observability/content_security_policy.rb b/app/controllers/concerns/observability/content_security_policy.rb
index eccd1e1e3ef..3865e3b606d 100644
--- a/app/controllers/concerns/observability/content_security_policy.rb
+++ b/app/controllers/concerns/observability/content_security_policy.rb
@@ -5,8 +5,14 @@ module Observability
extend ActiveSupport::Concern
included do
- content_security_policy do |p|
- next if p.directives.blank? || Gitlab::Observability.observability_url.blank?
+ content_security_policy_with_context do |p|
+ current_group = if defined?(group)
+ group
+ else
+ defined?(project) ? project&.group : nil
+ end
+
+ next if p.directives.blank? || !Gitlab::Observability.observability_enabled?(current_user, current_group)
default_frame_src = p.directives['frame-src'] || p.directives['default-src']
diff --git a/app/controllers/groups/observability_controller.rb b/app/controllers/groups/observability_controller.rb
index 3baa5e830ff..e0332ce6850 100644
--- a/app/controllers/groups/observability_controller.rb
+++ b/app/controllers/groups/observability_controller.rb
@@ -26,9 +26,7 @@ module Groups
end
def check_observability_allowed
- return render_404 unless Gitlab::Observability.observability_url.present?
-
- render_404 unless can?(current_user, :read_observability, @group)
+ render_404 unless Gitlab::Observability.observability_enabled?(current_user, group)
end
end
end
diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb
index 7ea15c830f3..1c546d70df9 100644
--- a/app/controllers/projects/merge_requests/diffs_controller.rb
+++ b/app/controllers/projects/merge_requests/diffs_controller.rb
@@ -46,8 +46,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
commit: commit,
diff_view: diff_view,
merge_ref_head_diff: render_merge_ref_head_diff?,
- pagination_data: diffs.pagination_data,
- merge_conflicts_in_diff: display_merge_conflicts_in_diff?
+ pagination_data: diffs.pagination_data
}
# NOTE: Any variables that would affect the resulting json needs to be added to the cache_context to avoid stale cache issues.
@@ -59,8 +58,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
params[:expanded],
params[:page],
params[:per_page],
- options[:merge_ref_head_diff],
- options[:merge_conflicts_in_diff]
+ options[:merge_ref_head_diff]
]
return unless stale?(etag: [cache_context + diff_options_hash.fetch(:paths, []), diffs])
@@ -84,8 +82,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
options = additional_attributes.merge(
only_context_commits: show_only_context_commits?,
- merge_ref_head_diff: render_merge_ref_head_diff?,
- merge_conflicts_in_diff: display_merge_conflicts_in_diff?
+ merge_ref_head_diff: render_merge_ref_head_diff?
)
render json: DiffsMetadataSerializer.new(project: @merge_request.project, current_user: current_user)
@@ -113,8 +110,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
options = additional_attributes.merge(
diff_view: "inline",
- merge_ref_head_diff: render_merge_ref_head_diff?,
- merge_conflicts_in_diff: display_merge_conflicts_in_diff?
+ merge_ref_head_diff: render_merge_ref_head_diff?
)
options[:context_commits] = @merge_request.recent_context_commits
@@ -242,8 +238,4 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter
.track_mr_diffs_single_file_action(merge_request: @merge_request, user: current_user)
end
-
- def display_merge_conflicts_in_diff?
- Feature.enabled?(:display_merge_conflicts_in_diff, @merge_request.project)
- end
end
diff --git a/app/serializers/diffs_entity.rb b/app/serializers/diffs_entity.rb
index 759d1e0f10a..5b30c0bb58c 100644
--- a/app/serializers/diffs_entity.rb
+++ b/app/serializers/diffs_entity.rb
@@ -74,7 +74,7 @@ class DiffsEntity < Grape::Entity
options.merge(
submodule_links: submodule_links,
code_navigation_path: code_navigation_path(diffs),
- conflicts: (conflicts(allow_tree_conflicts: true) if options[:merge_conflicts_in_diff])
+ conflicts: conflicts(allow_tree_conflicts: true)
)
)
end
diff --git a/app/serializers/diffs_metadata_entity.rb b/app/serializers/diffs_metadata_entity.rb
index ace5105dda5..e55f31a8376 100644
--- a/app/serializers/diffs_metadata_entity.rb
+++ b/app/serializers/diffs_metadata_entity.rb
@@ -6,7 +6,7 @@ class DiffsMetadataEntity < DiffsEntity
DiffFileMetadataEntity.represent(
diffs.raw_diff_files(sorted: true),
options.merge(
- conflicts: (conflicts(allow_tree_conflicts: true) if options[:merge_conflicts_in_diff])
+ conflicts: conflicts(allow_tree_conflicts: true)
)
)
end
diff --git a/app/serializers/paginated_diff_entity.rb b/app/serializers/paginated_diff_entity.rb
index b79a0937659..67f4014990c 100644
--- a/app/serializers/paginated_diff_entity.rb
+++ b/app/serializers/paginated_diff_entity.rb
@@ -17,7 +17,7 @@ class PaginatedDiffEntity < Grape::Entity
options.merge(
submodule_links: submodule_links,
code_navigation_path: code_navigation_path(diffs),
- conflicts: (conflicts(allow_tree_conflicts: true) if options[:merge_conflicts_in_diff])
+ conflicts: conflicts(allow_tree_conflicts: true)
)
)
end
diff --git a/config/feature_flags/development/display_merge_conflicts_in_diff.yml b/config/feature_flags/development/display_merge_conflicts_in_diff.yml
deleted file mode 100644
index 71146d9236b..00000000000
--- a/config/feature_flags/development/display_merge_conflicts_in_diff.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: display_merge_conflicts_in_diff
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45008
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/276918
-milestone: '13.5'
-type: development
-group: group::code review
-default_enabled: true
diff --git a/db/post_migrate/20221228210616_add_index_on_ci_runners_on_runner_type_and_id.rb b/db/post_migrate/20221228210616_add_index_on_ci_runners_on_runner_type_and_id.rb
new file mode 100644
index 00000000000..62c40b78945
--- /dev/null
+++ b/db/post_migrate/20221228210616_add_index_on_ci_runners_on_runner_type_and_id.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+class AddIndexOnCiRunnersOnRunnerTypeAndId < Gitlab::Database::Migration[2.1]
+ disable_ddl_transaction!
+
+ OLD_INDEX_NAME = 'index_ci_runners_on_runner_type'
+ NEW_INDEX_NAME = 'index_ci_runners_on_runner_type_and_id'
+
+ def up
+ add_concurrent_index :ci_runners, [:runner_type, :id], name: NEW_INDEX_NAME
+ remove_concurrent_index_by_name :ci_runners, OLD_INDEX_NAME
+ end
+
+ def down
+ add_concurrent_index :ci_runners, :runner_type, name: OLD_INDEX_NAME
+ remove_concurrent_index_by_name :ci_runners, NEW_INDEX_NAME
+ end
+end
diff --git a/db/schema_migrations/20221228210616 b/db/schema_migrations/20221228210616
new file mode 100644
index 00000000000..69ff81fe265
--- /dev/null
+++ b/db/schema_migrations/20221228210616
@@ -0,0 +1 @@
+a248f26495d2fab454538f8dd1c43e73ef4078f1822a900fce97b8c7f6df74c6 \ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index b252e564872..f549cb1c250 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -28999,7 +28999,7 @@ CREATE INDEX index_ci_runners_on_description_trigram ON ci_runners USING gin (de
CREATE INDEX index_ci_runners_on_locked ON ci_runners USING btree (locked);
-CREATE INDEX index_ci_runners_on_runner_type ON ci_runners USING btree (runner_type);
+CREATE INDEX index_ci_runners_on_runner_type_and_id ON ci_runners USING btree (runner_type, id);
CREATE INDEX index_ci_runners_on_token_expires_at_and_id_desc ON ci_runners USING btree (token_expires_at, id DESC);
diff --git a/doc/user/project/merge_requests/changes.md b/doc/user/project/merge_requests/changes.md
index f6e02dc0dfe..07ee2ef90c4 100644
--- a/doc/user/project/merge_requests/changes.md
+++ b/doc/user/project/merge_requests/changes.md
@@ -140,6 +140,7 @@ Files marked as viewed are not shown to you again unless either:
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/232484) in GitLab 13.5 [with a flag](../../../administration/feature_flags.md) named `display_merge_conflicts_in_diff`. Disabled by default.
> - [Enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/276918) in GitLab 15.7.
+> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/276918) in GitLab 15.8. Feature flag `display_merge_conflicts_in_diff` removed.
To avoid displaying the changes that are already on target branch in the diff,
we compare the merge request's source branch with HEAD of the target branch.
diff --git a/lib/gitlab/observability.rb b/lib/gitlab/observability.rb
index 8dde60a73be..8dbd2f41ccb 100644
--- a/lib/gitlab/observability.rb
+++ b/lib/gitlab/observability.rb
@@ -11,5 +11,9 @@ module Gitlab
'https://observe.gitlab.com'
end
+
+ def observability_enabled?(user, group)
+ Gitlab::Observability.observability_url.present? && Ability.allowed?(user, :read_observability, group)
+ end
end
end
diff --git a/lib/sidebars/groups/menus/observability_menu.rb b/lib/sidebars/groups/menus/observability_menu.rb
index 656142375af..8cdbfbd3342 100644
--- a/lib/sidebars/groups/menus/observability_menu.rb
+++ b/lib/sidebars/groups/menus/observability_menu.rb
@@ -23,7 +23,7 @@ module Sidebars
override :render?
def render?
- can?(context.current_user, :read_observability, context.group)
+ Gitlab::Observability.observability_enabled?(context.current_user, context.group)
end
private
diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb
index f4fd78730b6..4de724fd6d6 100644
--- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb
@@ -213,7 +213,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do
commit: nil,
latest_diff: true,
only_context_commits: false,
- merge_conflicts_in_diff: true,
merge_ref_head_diff: false
}
end
@@ -281,7 +280,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do
commit: nil,
latest_diff: true,
only_context_commits: false,
- merge_conflicts_in_diff: true,
merge_ref_head_diff: nil
}
end
@@ -303,33 +301,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do
commit: merge_request.diff_head_commit,
latest_diff: nil,
only_context_commits: false,
- merge_conflicts_in_diff: true,
- merge_ref_head_diff: nil
- }
- end
- end
- end
-
- context 'when display_merge_conflicts_in_diff is disabled' do
- subject { go }
-
- before do
- stub_feature_flags(display_merge_conflicts_in_diff: false)
- end
-
- it_behaves_like 'serializes diffs metadata with expected arguments' do
- let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiff }
- let(:expected_options) do
- {
- merge_request: merge_request,
- merge_request_diff: merge_request.merge_request_diff,
- merge_request_diffs: merge_request.merge_request_diffs,
- start_version: nil,
- start_sha: nil,
- commit: nil,
- latest_diff: true,
- only_context_commits: false,
- merge_conflicts_in_diff: false,
merge_ref_head_diff: nil
}
end
@@ -498,7 +469,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do
commit: nil,
diff_view: :inline,
merge_ref_head_diff: nil,
- merge_conflicts_in_diff: true,
pagination_data: {
total_pages: nil
}.merge(pagination_data)
@@ -617,21 +587,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do
it_behaves_like 'successful request'
end
- context 'when display_merge_conflicts_in_diff is disabled' do
- before do
- stub_feature_flags(display_merge_conflicts_in_diff: false)
- end
-
- subject { go }
-
- it_behaves_like 'serializes diffs with expected arguments' do
- let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
- let(:expected_options) { collection_arguments(total_pages: 20).merge(merge_conflicts_in_diff: false) }
- end
-
- it_behaves_like 'successful request'
- end
-
it_behaves_like 'forked project with submodules'
it_behaves_like 'cached diff collection'
diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb
index a8ef99eeaec..5b0c88e711d 100644
--- a/spec/finders/ci/runners_finder_spec.rb
+++ b/spec/finders/ci/runners_finder_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe Ci::RunnersFinder do
+RSpec.describe Ci::RunnersFinder, feature_category: :runner_fleet do
context 'admin' do
let_it_be(:admin) { create(:user, :admin) }
diff --git a/spec/lib/gitlab/observability_spec.rb b/spec/lib/gitlab/observability_spec.rb
index 2b1d22d9019..8068d2f8ec9 100644
--- a/spec/lib/gitlab/observability_spec.rb
+++ b/spec/lib/gitlab/observability_spec.rb
@@ -1,6 +1,6 @@
# frozen_string_literal: true
-require 'fast_spec_helper'
+require 'spec_helper'
RSpec.describe Gitlab::Observability do
describe '.observability_url' do
@@ -30,4 +30,39 @@ RSpec.describe Gitlab::Observability do
it { is_expected.to eq(observe_url) }
end
end
+
+ describe '.observability_enabled?' do
+ let_it_be(:group) { build(:user) }
+ let_it_be(:user) { build(:group) }
+
+ subject do
+ described_class.observability_enabled?(user, group)
+ end
+
+ it 'checks if read_observability ability is allowed for the given user and group' do
+ allow(Ability).to receive(:allowed?).and_return(true)
+
+ subject
+
+ expect(Ability).to have_received(:allowed?).with(user, :read_observability, group)
+ end
+
+ it 'returns true if the read_observability ability is allowed' do
+ allow(Ability).to receive(:allowed?).and_return(true)
+
+ expect(subject).to eq(true)
+ end
+
+ it 'returns false if the read_observability ability is not allowed' do
+ allow(Ability).to receive(:allowed?).and_return(false)
+
+ expect(subject).to eq(false)
+ end
+
+ it 'returns false if observability url is missing' do
+ allow(described_class).to receive(:observability_url).and_return("")
+
+ expect(subject).to eq(false)
+ end
+ end
end
diff --git a/spec/lib/sidebars/groups/menus/observability_menu_spec.rb b/spec/lib/sidebars/groups/menus/observability_menu_spec.rb
index 3a91b1aea2f..5b993cd6f28 100644
--- a/spec/lib/sidebars/groups/menus/observability_menu_spec.rb
+++ b/spec/lib/sidebars/groups/menus/observability_menu_spec.rb
@@ -20,23 +20,25 @@ RSpec.describe Sidebars::Groups::Menus::ObservabilityMenu do
allow(menu).to receive(:can?).and_call_original
end
- context 'when user can :read_observability' do
+ context 'when observability is enabled' do
before do
- allow(menu).to receive(:can?).with(user, :read_observability, group).and_return(true)
+ allow(Gitlab::Observability).to receive(:observability_enabled?).and_return(true)
end
it 'returns true' do
expect(menu.render?).to eq true
+ expect(Gitlab::Observability).to have_received(:observability_enabled?).with(user, group)
end
end
- context 'when user cannot :read_observability' do
+ context 'when observability is disabled' do
before do
- allow(menu).to receive(:can?).with(user, :read_observability, group).and_return(false)
+ allow(Gitlab::Observability).to receive(:observability_enabled?).and_return(false)
end
it 'returns false' do
expect(menu.render?).to eq false
+ expect(Gitlab::Observability).to have_received(:observability_enabled?).with(user, group)
end
end
end
diff --git a/spec/requests/projects/issues_controller_spec.rb b/spec/requests/projects/issues_controller_spec.rb
index a943bd6449c..67a73834f2d 100644
--- a/spec/requests/projects/issues_controller_spec.rb
+++ b/spec/requests/projects/issues_controller_spec.rb
@@ -8,10 +8,14 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do
let_it_be(:project) { issue.project }
let_it_be(:user) { issue.author }
+ shared_context 'group project issue' do
+ let_it_be(:project) { create :project, group: group }
+ let_it_be(:issue) { create :issue, project: project }
+ let_it_be(:user) { create(:user) }
+ end
+
describe 'GET #new' do
- before do
- login_as(user)
- end
+ include_context 'group project issue'
it_behaves_like "observability csp policy", described_class do
let(:tested_path) do
@@ -21,9 +25,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do
end
describe 'GET #show' do
- before do
- login_as(user)
- end
+ include_context 'group project issue'
it_behaves_like "observability csp policy", described_class do
let(:tested_path) do
@@ -60,7 +62,10 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do
end
let_it_be(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) }
- let_it_be(:discussion_reply) { create(:discussion_note_on_issue, noteable: issue, project: issue.project, in_reply_to: discussion) }
+ let_it_be(:discussion_reply) do
+ create(:discussion_note_on_issue, noteable: issue, project: issue.project, in_reply_to: discussion)
+ end
+
let_it_be(:state_event) { create(:resource_state_event, issue: issue) }
let_it_be(:discussion_2) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) }
let_it_be(:discussion_3) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) }
@@ -114,7 +119,8 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do
context 'when private project' do
let_it_be(:private_project) { create(:project, :private) }
- it_behaves_like 'authenticates sessionless user for the request spec', 'index atom', public_resource: false, ignore_metrics: true do
+ it_behaves_like 'authenticates sessionless user for the request spec', 'index atom', public_resource: false,
+ignore_metrics: true do
let(:url) { project_issues_url(private_project, format: :atom) }
before do
@@ -122,7 +128,8 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do
end
end
- it_behaves_like 'authenticates sessionless user for the request spec', 'calendar ics', public_resource: false, ignore_metrics: true do
+ it_behaves_like 'authenticates sessionless user for the request spec', 'calendar ics', public_resource: false,
+ignore_metrics: true do
let(:url) { project_issues_url(private_project, format: :ics) }
before do
diff --git a/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb b/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb
index 10e57970704..60223a30d28 100644
--- a/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb
+++ b/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb
@@ -35,7 +35,6 @@ RSpec.describe 'Merge Requests Context Commit Diffs', feature_category: :code_re
commit: nil,
diff_view: :inline,
merge_ref_head_diff: nil,
- merge_conflicts_in_diff: true,
pagination_data: {
total_pages: nil
}.merge(pagination_data)
diff --git a/spec/requests/projects/merge_requests/creations_spec.rb b/spec/requests/projects/merge_requests/creations_spec.rb
index 59e2047e1c7..e299d711cb1 100644
--- a/spec/requests/projects/merge_requests/creations_spec.rb
+++ b/spec/requests/projects/merge_requests/creations_spec.rb
@@ -26,14 +26,17 @@ RSpec.describe 'merge requests creations', feature_category: :code_review do
end
it_behaves_like "observability csp policy", Projects::MergeRequests::CreationsController do
+ let_it_be(:group) { create(:group) }
+ let_it_be(:user) { create(:user) }
+ let_it_be(:project) { create(:project, group: group) }
let(:tested_path) do
project_new_merge_request_path(project, merge_request: {
- title: 'Some feature',
- source_branch: 'fix',
- target_branch: 'feature',
- target_project: project,
- source_project: project
- })
+ title: 'Some feature',
+ source_branch: 'fix',
+ target_branch: 'feature',
+ target_project: project,
+ source_project: project
+ })
end
end
end
diff --git a/spec/requests/projects/merge_requests/diffs_spec.rb b/spec/requests/projects/merge_requests/diffs_spec.rb
index 858acac7f0d..dfdd372f8ad 100644
--- a/spec/requests/projects/merge_requests/diffs_spec.rb
+++ b/spec/requests/projects/merge_requests/diffs_spec.rb
@@ -33,7 +33,6 @@ RSpec.describe 'Merge Requests Diffs', feature_category: :code_review do
commit: nil,
diff_view: :inline,
merge_ref_head_diff: nil,
- merge_conflicts_in_diff: true,
pagination_data: {
total_pages: nil
}.merge(pagination_data)
@@ -112,17 +111,6 @@ RSpec.describe 'Merge Requests Diffs', feature_category: :code_review do
it_behaves_like 'serializes diffs with expected arguments'
end
- context 'with disabled display_merge_conflicts_in_diff feature' do
- let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
- let(:expected_options) { collection_arguments(total_pages: 20).merge(merge_conflicts_in_diff: false) }
-
- before do
- stub_feature_flags(display_merge_conflicts_in_diff: false)
- end
-
- it_behaves_like 'serializes diffs with expected arguments'
- end
-
context 'with diff_head option' do
subject { go(page: 0, per_page: 5, diff_head: true) }
diff --git a/spec/requests/projects/merge_requests_controller_spec.rb b/spec/requests/projects/merge_requests_controller_spec.rb
index aa7a32ef2cf..2e16529b20d 100644
--- a/spec/requests/projects/merge_requests_controller_spec.rb
+++ b/spec/requests/projects/merge_requests_controller_spec.rb
@@ -8,6 +8,12 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :source_code
let_it_be(:user) { merge_request.author }
describe 'GET #show' do
+ let_it_be(:group) { create(:group) }
+ let_it_be(:user) { create(:user) }
+ let_it_be(:project) { create :project, group: group }
+
+ let_it_be(:merge_request) { create :merge_request, source_project: project, author: user }
+
before do
login_as(user)
end
@@ -43,7 +49,10 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :source_code
describe 'GET #discussions' do
let_it_be(:discussion) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) }
- let_it_be(:discussion_reply) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, in_reply_to: discussion) }
+ let_it_be(:discussion_reply) do
+ create(:discussion_note_on_merge_request, noteable: merge_request, project: project, in_reply_to: discussion)
+ end
+
let_it_be(:state_event) { create(:resource_state_event, merge_request: merge_request) }
let_it_be(:discussion_2) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) }
let_it_be(:discussion_3) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) }
@@ -113,7 +122,8 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :source_code
context 'when private project' do
let_it_be(:private_project) { create(:project, :private) }
- it_behaves_like 'authenticates sessionless user for the request spec', 'index atom', public_resource: false, ignore_metrics: true do
+ it_behaves_like 'authenticates sessionless user for the request spec', 'index atom', public_resource: false,
+ ignore_metrics: true do
let(:url) { project_merge_requests_url(private_project, format: :atom) }
before do
diff --git a/spec/serializers/diffs_entity_spec.rb b/spec/serializers/diffs_entity_spec.rb
index ba40d538ccb..aa8e7275870 100644
--- a/spec/serializers/diffs_entity_spec.rb
+++ b/spec/serializers/diffs_entity_spec.rb
@@ -9,13 +9,11 @@ RSpec.describe DiffsEntity do
let(:request) { EntityRequest.new(project: project, current_user: user) }
let(:merge_request_diffs) { merge_request.merge_request_diffs }
- let(:merge_conflicts_in_diff) { false }
let(:options) do
{
request: request,
merge_request: merge_request,
- merge_request_diffs: merge_request_diffs,
- merge_conflicts_in_diff: merge_conflicts_in_diff
+ merge_request_diffs: merge_request_diffs
}
end
@@ -101,10 +99,9 @@ RSpec.describe DiffsEntity do
subject[:diff_files]
end
- context 'when merge_conflicts_in_diff is true' do
+ context 'when there are conflicts' do
let(:conflict_file) { double(path: diff_files.first.new_path, conflict_type: :both_modified) }
let(:conflicts) { double(conflicts: double(files: [conflict_file]), can_be_resolved_in_ui?: false) }
- let(:merge_conflicts_in_diff) { true }
before do
allow(merge_request).to receive(:cannot_be_merged?).and_return(true)
diff --git a/spec/serializers/diffs_metadata_entity_spec.rb b/spec/serializers/diffs_metadata_entity_spec.rb
index 04db576ffb5..415a0d8e450 100644
--- a/spec/serializers/diffs_metadata_entity_spec.rb
+++ b/spec/serializers/diffs_metadata_entity_spec.rb
@@ -9,7 +9,6 @@ RSpec.describe DiffsMetadataEntity do
let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
let(:merge_request_diffs) { merge_request.merge_request_diffs }
let(:merge_request_diff) { merge_request_diffs.last }
- let(:merge_conflicts_in_diff) { false }
let(:options) { {} }
let(:entity) do
@@ -18,8 +17,7 @@ RSpec.describe DiffsMetadataEntity do
options.merge(
request: request,
merge_request: merge_request,
- merge_request_diffs: merge_request_diffs,
- merge_conflicts_in_diff: merge_conflicts_in_diff
+ merge_request_diffs: merge_request_diffs
)
)
end
@@ -67,10 +65,9 @@ RSpec.describe DiffsMetadataEntity do
subject[:diff_files]
end
- context 'when merge_conflicts_in_diff is true' do
+ context 'when there are conflicts' do
let(:conflict_file) { double(path: raw_diff_files.first.new_path, conflict_type: :both_modified) }
let(:conflicts) { double(conflicts: double(files: [conflict_file]), can_be_resolved_in_ui?: false) }
- let(:merge_conflicts_in_diff) { true }
before do
allow(merge_request).to receive(:cannot_be_merged?).and_return(true)
diff --git a/spec/serializers/paginated_diff_entity_spec.rb b/spec/serializers/paginated_diff_entity_spec.rb
index 3d77beb9abc..29484d170f8 100644
--- a/spec/serializers/paginated_diff_entity_spec.rb
+++ b/spec/serializers/paginated_diff_entity_spec.rb
@@ -7,13 +7,11 @@ RSpec.describe PaginatedDiffEntity do
let(:request) { double('request', current_user: user) }
let(:merge_request) { create(:merge_request) }
let(:diff_batch) { merge_request.merge_request_diff.diffs_in_batch(2, 3, diff_options: nil) }
- let(:merge_conflicts_in_diff) { false }
let(:options) do
{
request: request,
merge_request: merge_request,
- pagination_data: diff_batch.pagination_data,
- merge_conflicts_in_diff: merge_conflicts_in_diff
+ pagination_data: diff_batch.pagination_data
}
end
@@ -43,10 +41,9 @@ RSpec.describe PaginatedDiffEntity do
subject[:diff_files]
end
- context 'when merge_conflicts_in_diff is true' do
+ context 'when there are conflicts' do
let(:conflict_file) { double(path: diff_files.first.new_path, conflict_type: :both_modified) }
let(:conflicts) { double(conflicts: double(files: [conflict_file]), can_be_resolved_in_ui?: false) }
- let(:merge_conflicts_in_diff) { true }
before do
allow(merge_request).to receive(:cannot_be_merged?).and_return(true)
diff --git a/spec/support/shared_examples/observability/csp_shared_examples.rb b/spec/support/shared_examples/observability/csp_shared_examples.rb
index 868d7023d14..0cd211f69eb 100644
--- a/spec/support/shared_examples/observability/csp_shared_examples.rb
+++ b/spec/support/shared_examples/observability/csp_shared_examples.rb
@@ -2,9 +2,17 @@
# Verifies that the proper CSP rules for Observabilty UI are applied to a given controller/path
#
-# The path under test needs to be declared with `let(:tested_path) { .. }` in the context including this example
+# It requires the following variables declared in the context including this example:
+#
+# - `tested_path`: the path under test
+# - `user`: the test user
+# - `group`: the test group
+#
+# e.g.
#
# ```
+# let_it_be(:group) { create(:group) }
+# let_it_be(:user) { create(:user) }
# it_behaves_like "observability csp policy" do
# let(:tested_path) { ....the path under test }
# end
@@ -33,6 +41,9 @@ RSpec.shared_examples 'observability csp policy' do |controller_class = describe
before do
setup_csp_for_controller(controller_class, csp, any_time: true)
+ group.add_developer(user)
+ login_as(user)
+ allow(Gitlab::Observability).to receive(:observability_enabled?).and_return(true)
end
subject do
@@ -48,6 +59,40 @@ RSpec.shared_examples 'observability csp policy' do |controller_class = describe
end
end
+ context 'when observability is disabled' do
+ let(:csp) do
+ ActionDispatch::ContentSecurityPolicy.new do |p|
+ p.frame_src 'https://something.test'
+ end
+ end
+
+ before do
+ allow(Gitlab::Observability).to receive(:observability_enabled?).and_return(false)
+ end
+
+ it 'does not add observability urls to the csp header' do
+ expect(subject).to include("frame-src https://something.test")
+ expect(subject).not_to include("#{observability_url} #{signin_url} #{oauth_url}")
+ end
+ end
+
+ context 'when checking if observability is enabled' do
+ let(:csp) do
+ ActionDispatch::ContentSecurityPolicy.new do |p|
+ p.frame_src 'https://something.test'
+ end
+ end
+
+ it 'check access for a given user and group' do
+ allow(Gitlab::Observability).to receive(:observability_enabled?)
+
+ get tested_path
+
+ expect(Gitlab::Observability).to have_received(:observability_enabled?)
+ .with(user, group).at_least(:once)
+ end
+ end
+
context 'when frame-src exists in the CSP config' do
let(:csp) do
ActionDispatch::ContentSecurityPolicy.new do |p|