Commit 5fc8b599 authored by Andreas Brandl's avatar Andreas Brandl

Merge branch 'rm-environments-in-mr-diffs' into 'master'

Remove obsolete view on environment button in MR diffs

See merge request gitlab-org/gitlab!77963
parents fc8e3d83 90f07756
......@@ -56,9 +56,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
@diff_notes_disabled = true
@environment = @merge_request.environments_for(current_user, latest: true).last
render json: { html: view_to_html_string('projects/merge_requests/creations/_diffs', diffs: @diffs, environment: @environment) }
render json: { html: view_to_html_string('projects/merge_requests/creations/_diffs', diffs: @diffs) }
end
def diff_for_path
......
......@@ -35,13 +35,11 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
diffs = @compare.diffs_in_batch(params[:page], params[:per_page], diff_options: diff_options_hash)
unfoldable_positions = @merge_request.note_positions_for_paths(diffs.diff_file_paths, current_user).unfoldable
environment = @merge_request.environments_for(current_user, latest: true).last
diffs.unfold_diff_files(unfoldable_positions)
diffs.write_cache
options = {
environment: environment,
merge_request: @merge_request,
commit: commit,
diff_view: diff_view,
......@@ -54,7 +52,6 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
# NOTE: Any variables that would affect the resulting json needs to be added to the cache_context to avoid stale cache issues.
cache_context = [
current_user&.cache_key,
environment&.cache_key,
unfoldable_positions.map(&:to_h),
diff_view,
params[:w],
......@@ -98,7 +95,6 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
# Deprecated: https://gitlab.com/gitlab-org/gitlab/issues/37735
def render_diffs
diffs = @compare.diffs(diff_options)
@environment = @merge_request.environments_for(current_user, latest: true).last
diffs.unfold_diff_files(note_positions.unfoldable)
diffs.write_cache
......@@ -175,7 +171,6 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
def additional_attributes
{
environment: @environment,
merge_request: @merge_request,
merge_request_diff: @merge_request_diff,
merge_request_diffs: @merge_request_diffs,
......
......@@ -14,8 +14,7 @@ module Environments
def execute
deployments =
if ref
deployments_query = params[:with_tags] ? 'ref = :ref OR tag IS TRUE' : 'ref = :ref'
Deployment.where(deployments_query, ref: ref.to_s)
Deployment.where(ref: ref.to_s)
elsif commit
Deployment.where(sha: commit.sha)
else
......
......@@ -1395,20 +1395,6 @@ class MergeRequest < ApplicationRecord
actual_head_pipeline.success?
end
def environments_for(current_user, latest: false)
return [] unless diff_head_commit
envs = Environments::EnvironmentsByDeploymentsFinder.new(target_project, current_user,
ref: target_branch, commit: diff_head_commit, with_tags: true, find_latest: latest).execute
if source_project
envs.concat Environments::EnvironmentsByDeploymentsFinder.new(source_project, current_user,
ref: source_branch, commit: diff_head_commit, find_latest: latest).execute
end
envs.uniq
end
##
# This method is for looking for active environments which created via pipelines for merge requests.
# Since deployments run on a merge request ref (e.g. `refs/merge-requests/:iid/head`),
......
......@@ -181,7 +181,7 @@ After you have the route mapping set up, it takes effect in the following locati
![View app file list in merge request widget](img/view_on_mr_widget.png)
- In the diff for a merge request, comparison, or commit.
- In the diff for a comparison or commit.
![View on environment button in merge request diff](img/view_on_env_mr.png)
......
......@@ -205,7 +205,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do
let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiff }
let(:expected_options) do
{
environment: nil,
merge_request: merge_request,
merge_request_diff: merge_request.merge_request_diff,
merge_request_diffs: merge_request.merge_request_diffs,
......@@ -280,7 +279,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do
let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiff }
let(:expected_options) do
{
environment: nil,
merge_request: merge_request,
merge_request_diff: merge_request.merge_request_diff,
merge_request_diffs: merge_request.merge_request_diffs,
......@@ -303,7 +301,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do
let(:collection) { Gitlab::Diff::FileCollection::Commit }
let(:expected_options) do
{
environment: nil,
merge_request: merge_request,
merge_request_diff: nil,
merge_request_diffs: merge_request.merge_request_diffs,
......@@ -330,7 +327,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do
let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiff }
let(:expected_options) do
{
environment: nil,
merge_request: merge_request,
merge_request_diff: merge_request.merge_request_diff,
merge_request_diffs: merge_request.merge_request_diffs,
......@@ -494,7 +490,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do
def collection_arguments(pagination_data = {})
{
environment: nil,
merge_request: merge_request,
commit: nil,
diff_view: :inline,
......
......@@ -48,26 +48,6 @@ RSpec.describe 'View on environment', :js do
let(:environment) { create(:environment, project: project, name: 'review/feature', external_url: 'http://feature.review.example.com') }
let!(:deployment) { create(:deployment, :success, environment: environment, ref: branch_name, sha: sha) }
context 'when visiting the diff of a merge request for the branch' do
let(:merge_request) { create(:merge_request, :simple, source_project: project, source_branch: branch_name) }
before do
sign_in(user)
visit diffs_project_merge_request_path(project, merge_request)
wait_for_requests
end
it 'has a "View on env" button' do
within '.diffs' do
text = 'View on feature.review.example.com'
url = 'http://feature.review.example.com/ruby/feature'
expect(page).to have_selector("a[title='#{text}'][href='#{url}']")
end
end
end
context 'when visiting a comparison for the branch' do
before do
sign_in(user)
......
......@@ -22,16 +22,6 @@ RSpec.describe Environments::EnvironmentsByDeploymentsFinder do
create(:deployment, :success, environment: environment_two, ref: 'v1.1.0', tag: true, sha: project.commit('HEAD~1').id)
end
it 'returns environment when with_tags is set' do
expect(described_class.new(project, user, ref: 'master', commit: commit, with_tags: true).execute)
.to contain_exactly(environment, environment_two)
end
it 'does not return environment when no with_tags is set' do
expect(described_class.new(project, user, ref: 'master', commit: commit).execute)
.to be_empty
end
it 'does not return environment when commit is not part of deployment' do
expect(described_class.new(project, user, ref: 'master', commit: project.commit('feature')).execute)
.to be_empty
......@@ -41,7 +31,7 @@ RSpec.describe Environments::EnvironmentsByDeploymentsFinder do
# This tests to ensure we don't call one CommitIsAncestor per environment
it 'only calls Gitaly twice when multiple environments are present', :request_store do
expect do
result = described_class.new(project, user, ref: 'master', commit: commit, with_tags: true, find_latest: true).execute
result = described_class.new(project, user, ref: 'v1.1.0', commit: commit, find_latest: true).execute
expect(result).to contain_exactly(environment_two)
end.to change { Gitlab::GitalyClient.get_request_count }.by(2)
......
......@@ -3492,84 +3492,6 @@ RSpec.describe MergeRequest, factory_default: :keep do
end
end
describe "#environments_for" do
let(:project) { create(:project, :repository) }
let(:user) { project.creator }
let(:merge_request) { create(:merge_request, source_project: project) }
let(:source_branch) { merge_request.source_branch }
let(:target_branch) { merge_request.target_branch }
let(:source_oid) { project.commit(source_branch).id }
let(:target_oid) { project.commit(target_branch).id }
before do
merge_request.source_project.add_maintainer(user)
merge_request.target_project.add_maintainer(user)
end
context 'with multiple environments' do
let(:environments) { create_list(:environment, 3, project: project) }
before do
create(:deployment, :success, environment: environments.first, ref: source_branch, sha: source_oid)
create(:deployment, :success, environment: environments.second, ref: target_branch, sha: target_oid)
end
it 'selects deployed environments' do
expect(merge_request.environments_for(user)).to contain_exactly(environments.first)
end
it 'selects latest deployed environment' do
latest_environment = create(:environment, project: project)
create(:deployment, :success, environment: latest_environment, ref: source_branch, sha: source_oid)
expect(merge_request.environments_for(user)).to eq([environments.first, latest_environment])
expect(merge_request.environments_for(user, latest: true)).to contain_exactly(latest_environment)
end
end
context 'with environments on source project' do
let(:source_project) { fork_project(project, nil, repository: true) }
let(:merge_request) do
create(:merge_request,
source_project: source_project, source_branch: 'feature',
target_project: project)
end
let(:source_environment) { create(:environment, project: source_project) }
before do
create(:deployment, :success, environment: source_environment, ref: 'feature', sha: merge_request.diff_head_sha)
end
it 'selects deployed environments', :sidekiq_might_not_need_inline do
expect(merge_request.environments_for(user)).to contain_exactly(source_environment)
end
context 'with environments on target project' do
let(:target_environment) { create(:environment, project: project) }
before do
create(:deployment, :success, environment: target_environment, tag: true, sha: merge_request.diff_head_sha)
end
it 'selects deployed environments', :sidekiq_might_not_need_inline do
expect(merge_request.environments_for(user)).to contain_exactly(source_environment, target_environment)
end
end
end
context 'without a diff_head_commit' do
before do
expect(merge_request).to receive(:diff_head_commit).and_return(nil)
end
it 'returns an empty array' do
expect(merge_request.environments_for(user)).to be_empty
end
end
end
describe "#environments" do
subject { merge_request.environments }
......
......@@ -31,7 +31,6 @@ RSpec.describe 'Merge Requests Context Commit Diffs' do
def collection_arguments(pagination_data = {})
{
environment: nil,
merge_request: merge_request,
commit: nil,
diff_view: :inline,
......
......@@ -29,7 +29,6 @@ RSpec.describe 'Merge Requests Diffs' do
def collection_arguments(pagination_data = {})
{
environment: nil,
merge_request: merge_request,
commit: nil,
diff_view: :inline,
......@@ -110,21 +109,6 @@ RSpec.describe 'Merge Requests Diffs' do
end
end
context 'with a new environment' do
let(:environment) do
create(:environment, :available, project: project)
end
let!(:deployment) do
create(:deployment, :success, environment: environment, ref: merge_request.source_branch)
end
it_behaves_like 'serializes diffs with expected arguments' do
let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
let(:expected_options) { collection_arguments(total_pages: 20).merge(environment: environment) }
end
end
context 'with disabled display_merge_conflicts_in_diff feature' do
before do
stub_feature_flags(display_merge_conflicts_in_diff: false)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment