diff --git a/app/components/rapid_diffs/app_component.html.haml b/app/components/rapid_diffs/app_component.html.haml index 622ceb854db4d8666bbf787cca961e679f5dbf07..ff5f84fa98a96b0651e2ca6a9c8cbdca3b7342f5 100644 --- a/app/components/rapid_diffs/app_component.html.haml +++ b/app/components/rapid_diffs/app_component.html.haml @@ -40,7 +40,7 @@ - if diffs_list? = diffs_list - else - = render RapidDiffs::DiffFileComponent.with_collection(diffs_slice, parallel_view: parallel_view?) + = render RapidDiffs::DiffFileComponent.with_collection(diffs_slice, parallel_view: parallel_view?, environment: environment) - if diffs_stream_url %div{ data: { stream_remaining_diffs: true } } - else diff --git a/app/components/rapid_diffs/app_component.rb b/app/components/rapid_diffs/app_component.rb index 9bf74b59e2132a2bef653d84aba06c18cb5ad38d..3c745bd9af176dc162146ffe8754e38886c254fd 100644 --- a/app/components/rapid_diffs/app_component.rb +++ b/app/components/rapid_diffs/app_component.rb @@ -8,7 +8,7 @@ class AppComponent < ViewComponent::Base attr_reader :presenter delegate :diffs_stream_url, :reload_stream_url, :diffs_stats_endpoint, :diff_files_endpoint, :diff_file_endpoint, - :should_sort_metadata_files?, :diffs_slice, :lazy?, to: :presenter + :should_sort_metadata_files?, :diffs_slice, :lazy?, :environment, to: :presenter delegate :diff_view, :current_user, to: :helpers diff --git a/app/components/rapid_diffs/diff_file_component.rb b/app/components/rapid_diffs/diff_file_component.rb index 6735be9229a6d9de43dd9fca5125700bb051be23..00b7420524f7e30ccc7c829c7f516e74cfdcf4e2 100644 --- a/app/components/rapid_diffs/diff_file_component.rb +++ b/app/components/rapid_diffs/diff_file_component.rb @@ -7,9 +7,10 @@ class DiffFileComponent < ViewComponent::Base renders_one :header - def initialize(diff_file:, parallel_view: false) + def initialize(diff_file:, parallel_view: false, environment: nil) @diff_file = diff_file @parallel_view = parallel_view + @environment = environment end def id @@ -46,7 +47,10 @@ def viewer_component_instance end def default_header - render RapidDiffs::DiffFileHeaderComponent.new(diff_file: @diff_file) + render RapidDiffs::DiffFileHeaderComponent.new( + diff_file: @diff_file, + environment: @environment + ) end # enables virtual rendering through content-visibility: auto, significantly boosts client performance diff --git a/app/components/rapid_diffs/diff_file_header_component.rb b/app/components/rapid_diffs/diff_file_header_component.rb index 97092301a70bc4dcd5903be444dd39f5bd9c2742..4406e3da777f8e41872e6df2c8abf0e2db04ae94 100644 --- a/app/components/rapid_diffs/diff_file_header_component.rb +++ b/app/components/rapid_diffs/diff_file_header_component.rb @@ -5,9 +5,10 @@ class DiffFileHeaderComponent < ViewComponent::Base include ButtonHelper include DiffHelper - def initialize(diff_file:, additional_menu_items: []) + def initialize(diff_file:, additional_menu_items: [], environment: nil) @diff_file = diff_file @additional_menu_items = additional_menu_items + @environment = environment end def file_title_chunks @@ -38,10 +39,11 @@ def copy_path_button def menu_items [ view_file_menu_item, + view_on_environment_menu_item, *@additional_menu_items ] - .filter_map { |item| item unless item.nil? } - .sort_by { |item| item[:position] || Float::INFINITY } + .compact + .sort_by { |item| item[:position] || Float::INFINITY } end def heading_id @@ -92,5 +94,25 @@ def view_file_menu_item position: 1 } end + + def view_on_environment_menu_item + return unless @environment + + file_path = @diff_file.new_path || @diff_file.old_path + commit_sha = @diff_file.content_sha + + environment_path = @environment.external_url_for(file_path, commit_sha) + return unless environment_path + + { + text: helpers.safe_format( + s_('RapidDiffs|View on %{environment}'), + environment: @environment.formatted_external_url + ), + href: environment_path, + extraAttrs: { target: '_blank', rel: 'noopener noreferrer' }, + position: 2 + } + end end end diff --git a/app/controllers/concerns/rapid_diffs/resource.rb b/app/controllers/concerns/rapid_diffs/resource.rb index afa0accfa219d1b008d6b93313d10e427a7a9e6e..f2f6964be8b85cc6cf3fef9ad71abca74d551714 100644 --- a/app/controllers/concerns/rapid_diffs/resource.rb +++ b/app/controllers/concerns/rapid_diffs/resource.rb @@ -53,8 +53,10 @@ def diffs_resource(options = {}) raise NotImplementedError end + attr_reader :environment + def diff_file_component(base_args) - ::RapidDiffs::DiffFileComponent.new(**base_args) + ::RapidDiffs::DiffFileComponent.new(**base_args, environment: environment) end def find_diff_file(extra_options, old_path, new_path) diff --git a/app/controllers/concerns/rapid_diffs/streaming_resource.rb b/app/controllers/concerns/rapid_diffs/streaming_resource.rb index cfc4f0545124aa5ccecfe2f8c255175b9c6c6ca2..0f2f059bee9829d5952f80c624f752949033f3d1 100644 --- a/app/controllers/concerns/rapid_diffs/streaming_resource.rb +++ b/app/controllers/concerns/rapid_diffs/streaming_resource.rb @@ -85,12 +85,20 @@ def stream_diff_collection(options, view_context) response.stream.write(diff_files_collection(skipped).render_in(view_context)) unless skipped.empty? end + attr_reader :environment + def diff_file_component(diff_file) - ::RapidDiffs::DiffFileComponent.new(diff_file: diff_file, parallel_view: view == :parallel) + ::RapidDiffs::DiffFileComponent.new( + diff_file: diff_file, + parallel_view: view == :parallel, + environment: environment) end def diff_files_collection(diff_files) - ::RapidDiffs::DiffFileComponent.with_collection(diff_files, parallel_view: view == :parallel) + ::RapidDiffs::DiffFileComponent.with_collection( + diff_files, + parallel_view: view == :parallel, + environment: environment) end def stream_diff_blobs(options, view_context) diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 84d52244b2ff63852c028ae8c4d3d800f9d128c0..42b0e94fef767c5b824d0f6e5b61ac0505aa8d4a 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -17,6 +17,8 @@ class Projects::CommitController < Projects::ApplicationController before_action :authorize_read_pipeline!, only: [:pipelines] before_action :commit before_action :define_commit_vars, only: [:show, :diff_for_path, :diff_files, :pipelines, :merge_requests] + before_action :define_environment, + only: [:show, :rapid_diffs, :diff_for_path, :diff_files, :pipelines, :merge_requests] before_action :define_commit_box_vars, only: [:show, :pipelines, :rapid_diffs] before_action :define_note_vars, only: [:show, :diff_for_path, :diff_files, :discussions] before_action :authorize_edit_tree!, only: [:revert, :cherry_pick] @@ -172,7 +174,8 @@ def rapid_diffs diff_view, commit_diff_options, nil, - current_user + current_user, + define_environment ) show @@ -224,12 +227,6 @@ def define_commit_vars return git_not_found! unless commit @diffs = commit.diffs(commit_diff_options) - @environment = ::Environments::EnvironmentsByDeploymentsFinder.new( - @project, - current_user, - commit: @commit, - find_latest: true - ).execute.last end # rubocop: disable CodeReuse/ActiveRecord @@ -318,6 +315,15 @@ def complete_diff_path def email_format_path project_commit_path(project, commit, format: :patch) end + + def define_environment + @environment ||= ::Environments::EnvironmentsByDeploymentsFinder.new( + @project, + current_user, + commit: @commit, + find_latest: true + ).execute.last + end end Projects::CommitController.prepend_mod_with('Projects::CommitController') diff --git a/app/controllers/projects/commit_diffs_stream_controller.rb b/app/controllers/projects/commit_diffs_stream_controller.rb index 387511885a35150b6cfb220f4af2ed6b41484972..7c584150547bdf05ed46c59743e27c0fd6a9170e 100644 --- a/app/controllers/projects/commit_diffs_stream_controller.rb +++ b/app/controllers/projects/commit_diffs_stream_controller.rb @@ -4,6 +4,8 @@ module Projects class CommitDiffsStreamController < Projects::CommitController include RapidDiffs::StreamingResource + before_action :define_environment + private def resource diff --git a/app/controllers/projects/compare_diffs_stream_controller.rb b/app/controllers/projects/compare_diffs_stream_controller.rb index f73e9b91c5059f3a5e7d67d460a4426ab4287a2a..ccfa120ebcfe2795ab9fd1ed39bb3aecf13ca9a4 100644 --- a/app/controllers/projects/compare_diffs_stream_controller.rb +++ b/app/controllers/projects/compare_diffs_stream_controller.rb @@ -4,6 +4,8 @@ module Projects class CompareDiffsStreamController < Projects::CompareController include RapidDiffs::StreamingResource + before_action :define_environment + private def resource diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 43695d948ff26bb056aba539ad9d07656befc9c4..692df57cb201380e39709ab48483ee18e0de55b4 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -279,7 +279,7 @@ def view_on_environment_button(commit_sha, diff_new_path, environment) button_options: { rel: 'noopener noreferrer', title: "View on #{environment.formatted_external_url}", - class: 'has-tooltip', + class: 'has-tooltip gl-ml-3', data: { container: 'body' } } ) diff --git a/app/presenters/rapid_diffs/base_presenter.rb b/app/presenters/rapid_diffs/base_presenter.rb index e41a63fb02bf313b21ef9256c847ed175252f639..7dc6fc6a82ecc64fee65d05a5ee0f725a3bca856 100644 --- a/app/presenters/rapid_diffs/base_presenter.rb +++ b/app/presenters/rapid_diffs/base_presenter.rb @@ -2,13 +2,16 @@ module RapidDiffs class BasePresenter < Gitlab::View::Presenter::Delegated - def initialize(subject, diff_view, diff_options, request_params = nil) + def initialize(subject, diff_view, diff_options, request_params = nil, environment = nil) super(subject) @diff_view = diff_view @diff_options = diff_options @request_params = request_params + @environment = environment end + attr_reader :environment + def diffs_stream_url return reload_stream_url(diff_view: @diff_view) if offset == 0 return if offset.nil? || offset >= diffs_count diff --git a/app/presenters/rapid_diffs/commit_presenter.rb b/app/presenters/rapid_diffs/commit_presenter.rb index 24fef6988dd945d355a82e28319f8a604a09f168..ec9be55bd768e213e53d1b419b98acc30a71ae4f 100644 --- a/app/presenters/rapid_diffs/commit_presenter.rb +++ b/app/presenters/rapid_diffs/commit_presenter.rb @@ -6,8 +6,8 @@ class CommitPresenter < BasePresenter presents ::Commit, as: :resource - def initialize(subject, diff_view, diff_options, request_params = nil, current_user = nil) - super(subject, diff_view, diff_options, request_params) + def initialize(subject, diff_view, diff_options, request_params = nil, current_user = nil, environment = nil) + super(subject, diff_view, diff_options, request_params, environment) @current_user = current_user end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9053489adbb0e4d1fc225df2c943424432eab98d..e19356f41f80fb89b9a708ee2a265805b1020c6d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -54707,6 +54707,9 @@ msgstr "" msgid "RapidDiffs|View file at %{codeStart}%{commit}%{codeEnd}" msgstr "" +msgid "RapidDiffs|View on %{environment}" +msgstr "" + msgid "Rate Limits" msgstr "" diff --git a/spec/components/rapid_diffs/app_component_spec.rb b/spec/components/rapid_diffs/app_component_spec.rb index 354296e7f53f5b0d1cf720f881782b5e939b1857..5607b8a9b8cca81b7920f32da3662956cecd37e7 100644 --- a/spec/components/rapid_diffs/app_component_spec.rb +++ b/spec/components/rapid_diffs/app_component_spec.rb @@ -27,6 +27,7 @@ diff_files_endpoint: diff_files_endpoint, diff_file_endpoint: diff_file_endpoint, should_sort_metadata_files?: should_sort_metadata_files, + environment: nil, lazy?: lazy ) end diff --git a/spec/components/rapid_diffs/commit_app_component_spec.rb b/spec/components/rapid_diffs/commit_app_component_spec.rb index 969298d6d35028e12feaea5d5f51df3f36091e38..90d4882332171e68ccebd43cc53d79a3ad257d95 100644 --- a/spec/components/rapid_diffs/commit_app_component_spec.rb +++ b/spec/components/rapid_diffs/commit_app_component_spec.rb @@ -23,7 +23,8 @@ register_path: register_path, sign_in_path: sign_in_path, report_abuse_path: report_abuse_path, - markdown_docs_path: markdown_docs_path + markdown_docs_path: markdown_docs_path, + environment: nil ) end diff --git a/spec/components/rapid_diffs/diff_file_component_spec.rb b/spec/components/rapid_diffs/diff_file_component_spec.rb index e693c821bfe3460252c4f1a85f54e93a021cf290..655383e49276de3444609086eb073aef59dcb71b 100644 --- a/spec/components/rapid_diffs/diff_file_component_spec.rb +++ b/spec/components/rapid_diffs/diff_file_component_spec.rb @@ -13,7 +13,8 @@ it 'renders the default header when no custom header is provided' do allow_next_instance_of( RapidDiffs::DiffFileHeaderComponent, - diff_file: diff_file + diff_file: diff_file, + environment: nil ) do |instance| allow(instance).to receive(:render_in).and_return('diff-file-header') end @@ -32,6 +33,25 @@ expect(result.css('.custom-header').text).to eq('Custom Header') end + + context 'with environment' do + let_it_be(:project) { build_stubbed(:project, :repository) } + let_it_be(:environment) { build_stubbed(:environment, project: project) } + + it 'renders the default header with environment when no custom header is provided' do + allow_next_instance_of( + RapidDiffs::DiffFileHeaderComponent, + diff_file: diff_file, + environment: environment + ) do |instance| + allow(instance).to receive(:render_in).and_return('diff-file-header-with-env') + end + + result = render_component(environment: environment) + + expect(result.to_html).to include('diff-file-header-with-env') + end + end end def render_component(**args, &block) diff --git a/spec/components/rapid_diffs/diff_file_header_component_spec.rb b/spec/components/rapid_diffs/diff_file_header_component_spec.rb index 0491f88a77fde3e163f1e408fe62b244accbb03f..1c07e2309feefad0f568950e701ad22005f69ec7 100644 --- a/spec/components/rapid_diffs/diff_file_header_component_spec.rb +++ b/spec/components/rapid_diffs/diff_file_header_component_spec.rb @@ -190,6 +190,80 @@ expect(item).not_to have_key('position') end end + + context 'with environment' do + let(:project) { diff_file.repository.project } + let(:environment) { build(:environment, project: project, external_url: 'https://test.example.com') } + let(:environment_path) { "https://test.example.com/#{diff_file.new_path}" } + + before do + allow(environment).to receive(:formatted_external_url).and_return('test.example.com') + allow(environment).to receive(:external_url_for) + .with(diff_file.new_path, content_sha) + .and_return(environment_path) + end + + it 'renders "View on environment" menu item' do + render_component(environment: environment) + + options_menu_items = Gitlab::Json.parse(page.find('script', visible: false).text) + + view_on_env_item = options_menu_items.find { |item| item['text']&.include?('View on') } + + expect(view_on_env_item).not_to be_nil + expect(view_on_env_item['text']).to include('test.example.com') + expect(view_on_env_item['href']).to eq(environment_path) + expect(view_on_env_item['extraAttrs']['target']).to eq('_blank') + expect(view_on_env_item['extraAttrs']['rel']).to eq('noopener noreferrer') + end + + context 'when file is renamed' do + before do + allow(diff_file).to receive(:new_path).and_return('new/file/path.rb') + allow(diff_file).to receive(:old_path).and_return('old/file/path.rb') + allow(environment).to receive(:external_url_for) do |path, _sha| + path == diff_file.new_path ? environment_path : nil + end + end + + it 'uses new_path for environment URL generation' do + render_component(environment: environment) + + expect(page).to have_content('View on test.example.com') + expect(page).to have_content(environment_path) + end + end + + context 'when environment has no route map for the file' do + before do + allow(environment).to receive(:external_url_for) + .with(diff_file.new_path, content_sha) + .and_return(nil) + end + + it 'does not render "View on environment" menu item' do + render_component(environment: environment) + + options_menu_items = Gitlab::Json.parse(page.find('script', visible: false).text) + + view_on_env_item = options_menu_items.find { |item| item['text']&.include?('View on') } + + expect(view_on_env_item).to be_nil + end + end + end + + context 'without environment' do + it 'does not render "View on environment" menu item when environment is nil' do + render_component(environment: nil) + + options_menu_items = Gitlab::Json.parse(page.find('script', visible: false).text) + + view_on_env_item = options_menu_items.find { |item| item['text']&.include?('View on') } + + expect(view_on_env_item).to be_nil + end + end end def create_instance(**args) diff --git a/spec/controllers/concerns/rapid_diffs/resource_spec.rb b/spec/controllers/concerns/rapid_diffs/resource_spec.rb index 0b025499769518dce3d9391c75183038a33a74c9..775d0d8a587b6b21aa0843bda5a4752b67cc6b4b 100644 --- a/spec/controllers/concerns/rapid_diffs/resource_spec.rb +++ b/spec/controllers/concerns/rapid_diffs/resource_spec.rb @@ -61,12 +61,23 @@ def with_custom_diff_options describe '#diff_file_component' do it 'initializes a DiffFileComponent with the given arguments' do - args = { parallel_view: :parallel } + args = { diff_file: instance_double(Gitlab::Git::Diff), parallel_view: :parallel } - expect(RapidDiffs::DiffFileComponent).to receive(:new).with(**args) + expect(RapidDiffs::DiffFileComponent).to receive(:new).with(**args, environment: nil) controller.new.call_diff_file_component(args) end + + context 'when environment is nil' do + it 'creates a DiffFileComponent with nil environment' do + base_args = { diff_file: instance_double(Gitlab::Git::Diff) } + + expect(::RapidDiffs::DiffFileComponent).to receive(:new) + .with(**base_args, environment: nil) + + controller.new.call_diff_file_component(base_args) + end + end end describe '#find_diff_file' do @@ -88,4 +99,22 @@ def with_custom_diff_options expect(controller_instance.call_find_diff_file(extra_options, old_path, new_path)).to eq(diff_file) end end + + describe '#environment' do + it 'returns nil by default' do + controller_instance = controller.new + + expect(controller_instance.send(:environment)).to be_nil + end + + context 'when environment instance variable is set' do + it 'returns the environment' do + controller_instance = controller.new + environment = instance_double(Environment) + controller_instance.instance_variable_set(:@environment, environment) + + expect(controller_instance.send(:environment)).to eq(environment) + end + end + end end diff --git a/spec/controllers/concerns/rapid_diffs/streaming_resource_spec.rb b/spec/controllers/concerns/rapid_diffs/streaming_resource_spec.rb index f1db1e84dcee1bb211d326856b5fcdde40897368..38cf553a5d0d20e8bc818f88b34bce91bf90ab56 100644 --- a/spec/controllers/concerns/rapid_diffs/streaming_resource_spec.rb +++ b/spec/controllers/concerns/rapid_diffs/streaming_resource_spec.rb @@ -49,6 +49,150 @@ def diff_options end end + describe '#environment' do + let(:controller_instance) { controller.new } + + context 'when environment is set' do + let(:environment) { build(:environment) } + + before do + controller_instance.instance_variable_set(:@environment, environment) + end + + it 'returns the environment' do + expect(controller_instance.send(:environment)).to eq(environment) + end + end + + context 'when environment is not set' do + it 'returns nil' do + expect(controller_instance.send(:environment)).to be_nil + end + end + end + + describe '#diff_file_component' do + let(:controller_instance) { controller.new } + let(:diff_file) { build(:diff_file) } + + before do + allow(controller_instance).to receive_message_chain(:helpers, :diff_view).and_return(:inline) + end + + context 'when environment is nil' do + it 'creates a DiffFileComponent with nil environment' do + expect(::RapidDiffs::DiffFileComponent).to receive(:new) + .with( + diff_file: diff_file, + parallel_view: false, + environment: nil + ) + + controller_instance.send(:diff_file_component, diff_file) + end + end + + context 'when environment is set' do + let(:environment) { build(:environment) } + + before do + controller_instance.instance_variable_set(:@environment, environment) + end + + it 'creates a DiffFileComponent with the environment' do + expect(::RapidDiffs::DiffFileComponent).to receive(:new) + .with( + diff_file: diff_file, + parallel_view: false, + environment: environment + ) + + controller_instance.send(:diff_file_component, diff_file) + end + end + + context 'when view is parallel' do + let(:environment) { build(:environment) } + + before do + allow(controller_instance).to receive_message_chain(:helpers, :diff_view).and_return(:parallel) + controller_instance.instance_variable_set(:@environment, environment) + end + + it 'creates a DiffFileComponent with parallel_view: true' do + expect(::RapidDiffs::DiffFileComponent).to receive(:new) + .with( + diff_file: diff_file, + parallel_view: true, + environment: environment + ) + + controller_instance.send(:diff_file_component, diff_file) + end + end + end + + describe '#diff_files_collection' do + let(:controller_instance) { controller.new } + let(:diff_files) { [build(:diff_file), build(:diff_file)] } + + before do + allow(controller_instance).to receive_message_chain(:helpers, :diff_view).and_return(:inline) + end + + context 'when environment is nil' do + it 'creates a DiffFileComponent collection with nil environment' do + expect(::RapidDiffs::DiffFileComponent).to receive(:with_collection) + .with( + diff_files, + parallel_view: false, + environment: nil + ) + + controller_instance.send(:diff_files_collection, diff_files) + end + end + + context 'when environment is set' do + let(:environment) { build(:environment) } + + before do + controller_instance.instance_variable_set(:@environment, environment) + end + + it 'creates a DiffFileComponent collection with the environment' do + expect(::RapidDiffs::DiffFileComponent).to receive(:with_collection) + .with( + diff_files, + parallel_view: false, + environment: environment + ) + + controller_instance.send(:diff_files_collection, diff_files) + end + end + + context 'when view is parallel' do + let(:environment) { build(:environment) } + + before do + allow(controller_instance).to receive_message_chain(:helpers, :diff_view).and_return(:parallel) + controller_instance.instance_variable_set(:@environment, environment) + end + + it 'creates a DiffFileComponent collection with parallel_view: true' do + expect(::RapidDiffs::DiffFileComponent).to receive(:with_collection) + .with( + diff_files, + parallel_view: true, + environment: environment + ) + + controller_instance.send(:diff_files_collection, diff_files) + end + end + end + describe '#diffs' do let(:controller_instance) { controller.new } let(:mock_resource) { instance_double(::Commit) } diff --git a/spec/features/projects/view_on_env_spec.rb b/spec/features/projects/view_on_env_spec.rb index a790a64ee8c5e8016752f53f6ab3da1faf797464..0064b9052dcf1361b21a1ad108139c0e53151e92 100644 --- a/spec/features/projects/view_on_env_spec.rb +++ b/spec/features/projects/view_on_env_spec.rb @@ -51,6 +51,28 @@ context 'with legacy diffs' do before do stub_feature_flags(rapid_diffs_on_compare_show: false) + sign_in(user) + visit page_path + wait_for_requests + end + + where(:page_path) do + [ + lazy { project_compare_path(project, from: 'master', to: branch_name) }, + lazy { project_commit_path(project, sha) } + ] + end + + with_them do + it 'has a "View on env" button' do + expect(page).to have_link('View on feature.review.example.com', href: 'http://feature.review.example.com/ruby/feature') + end + end + end + + context 'with rapid diffs' do + before do + stub_feature_flags(rapid_diffs_on_compare_show: true) end context 'when visiting a comparison for the branch' do @@ -62,22 +84,45 @@ wait_for_requests end - it 'has a "View on env" button' do - expect(page).to have_link('View on feature.review.example.com', href: 'http://feature.review.example.com/ruby/feature') + it 'has a "View on env" button in the file header menu' do + diff_file = all_by_testid('rd-diff-file').find do |file| + file.has_text?(file_path) + end + diff_file.find('button:has([data-testid="ellipsis_v-icon"])').click + + expect(page).to have_link( + 'View on feature.review.example.com', + href: 'http://feature.review.example.com/ruby/feature' + ) end end - context 'when visiting a comparison for the commit' do + context 'when visiting the commit' do before do sign_in(user) - visit project_compare_path(project, from: 'master', to: sha) + visit project_commit_path(project, sha, rapid_diffs: true) wait_for_requests end - it 'has a "View on env" button' do - expect(page).to have_link('View on feature.review.example.com', href: 'http://feature.review.example.com/ruby/feature') + it 'has a "View on env" button in the file header menu' do + first_file = find_by_testid('rd-diff-file') + first_file.find('button:has([data-testid="ellipsis_v-icon"])').click + + expect(page).to have_link( + 'View on feature.review.example.com', + href: 'http://feature.review.example.com/ruby/feature' + ) + end + + it 'opens the environment URL in a new tab' do + first_file = find_by_testid('rd-diff-file') + first_file.find('button:has([data-testid="ellipsis_v-icon"])').click + + link = page.find_link('View on feature.review.example.com') + expect(link[:target]).to eq('_blank') + expect(link[:rel]).to include('noopener') end end end @@ -111,20 +156,6 @@ expect(page).to have_link('View on feature.review.example.com', href: 'http://feature.review.example.com/ruby/feature') end end - - context 'when visiting the commit' do - before do - sign_in(user) - - visit project_commit_path(project, sha) - - wait_for_requests - end - - it 'has a "View on env" button' do - expect(page).to have_link('View on feature.review.example.com', href: 'http://feature.review.example.com/ruby/feature') - end - end end end end diff --git a/spec/presenters/rapid_diffs/base_presenter_spec.rb b/spec/presenters/rapid_diffs/base_presenter_spec.rb index 2b0fc202fa44bfc66460f4b38a44e88721928430..604e4a84e4bfcdd4e7ac36861cc506d263da3d3c 100644 --- a/spec/presenters/rapid_diffs/base_presenter_spec.rb +++ b/spec/presenters/rapid_diffs/base_presenter_spec.rb @@ -3,7 +3,23 @@ require 'spec_helper' RSpec.describe RapidDiffs::BasePresenter, feature_category: :source_code_management do - subject(:presenter) { described_class.new(Class.new, :inline, {}) } + let(:diff_view) { :inline } + let(:diff_options) { {} } + let(:environment) { nil } + + subject(:presenter) { described_class.new(Class.new, diff_view, diff_options, nil, environment) } + + describe '#environment' do + subject(:method) { presenter.environment } + + it { is_expected.to be_nil } + + context 'when environment is provided' do + let(:environment) { build(:environment) } + + it { is_expected.to eq(environment) } + end + end describe 'abstract methods' do it 'raises a NotImplementedError for #diffs_stats_endpoint' do diff --git a/spec/requests/projects/commit_diffs_stream_controller_spec.rb b/spec/requests/projects/commit_diffs_stream_controller_spec.rb index e636f7b34b9d80cbc0c57b10645d3e6285b15ec3..3a505a3a465a089ed1f7cedabe8e7b50a1e2f465 100644 --- a/spec/requests/projects/commit_diffs_stream_controller_spec.rb +++ b/spec/requests/projects/commit_diffs_stream_controller_spec.rb @@ -38,5 +38,31 @@ def go(**extra_params) include_examples 'diffs stream tests' include_examples 'with diffs_blobs param' + + context 'with environment' do + let(:environment) { create(:environment, project: project, external_url: 'https://example.com') } + + before do + allow_next_instance_of(Environments::EnvironmentsByDeploymentsFinder) do |finder| + allow(finder).to receive(:execute).and_return([environment]) + end + + allow(project).to receive(:public_path_for_source_path).and_return('public/file.html') + end + + it 'includes environment link in response' do + go + + expect(response.body).to include(environment.formatted_external_url) + end + end + + context 'without environment' do + it 'does not include environment link in response' do + go + + expect(response.body).not_to include('View on') + end + end end end diff --git a/spec/requests/projects/compare_diffs_stream_controller_spec.rb b/spec/requests/projects/compare_diffs_stream_controller_spec.rb index c12b2e859acd7294ed7cea18fe8840481b475ed9..a64c4968b814e5815b2e0f99d9fa9527fd5e81d0 100644 --- a/spec/requests/projects/compare_diffs_stream_controller_spec.rb +++ b/spec/requests/projects/compare_diffs_stream_controller_spec.rb @@ -53,5 +53,43 @@ def go(**extra_params) include_examples 'diffs stream tests' include_examples 'with diffs_blobs param' + + context 'with environment' do + let(:environment) { create(:environment, project: project, external_url: 'https://example.com') } + + before do + create(:deployment, :success, environment: environment, project: project, sha: compare.head_commit_sha) + + allow_next_instance_of(Environments::EnvironmentsByDeploymentsFinder) do |finder| + allow(finder).to receive(:execute).and_return([environment]) + end + + allow(project).to receive(:public_path_for_source_path).and_return('public/file.html') + end + + it 'includes environment link in response' do + go + + expect(response.body).to include('View on') + expect(response.body).to include(environment.formatted_external_url) + end + end + + context 'without environment' do + before do + create(:deployment, :success, environment: create(:environment, project: project), + project: project, sha: compare.head_commit_sha) + + allow_next_instance_of(Environments::EnvironmentsByDeploymentsFinder) do |finder| + allow(finder).to receive(:execute).and_return([]) + end + end + + it 'does not include environment link in response' do + go + + expect(response.body).not_to include('View on') + end + end end end