diff --git a/app/presenters/blob_presenter.rb b/app/presenters/blob_presenter.rb index cd473152b41c144af98fe21e2913463ea854fa39..bc12d210334bec451d05d09fae4b77866054a915 100644 --- a/app/presenters/blob_presenter.rb +++ b/app/presenters/blob_presenter.rb @@ -86,7 +86,7 @@ def gitpod_blob_url end def find_file_path - url_helpers.project_find_file_path(project, blob.commit_id) + url_helpers.project_find_file_path(project, commit_id, ref_type: ref_type) end def blame_path @@ -131,13 +131,13 @@ def fork_and_view_path end def can_modify_blob? - super(blob, project, blob.commit_id) + super(blob, project, commit_id) end def can_current_user_push_to_branch? - return false unless current_user && project.repository.branch_exists?(blob.commit_id) + return false unless current_user && project.repository.branch_exists?(commit_id) - user_access(project).can_push_to_branch?(blob.commit_id) + user_access(project).can_push_to_branch?(commit_id) end def archived? @@ -145,7 +145,7 @@ def archived? end def ide_edit_path - super(project, blob.commit_id, blob.path) + super(project, commit_id, blob.path) end def external_storage_url @@ -159,7 +159,7 @@ def code_navigation_path end def project_blob_path_root - project_blob_path(project, blob.commit_id) + project_blob_path(project, commit_id) end private @@ -181,7 +181,7 @@ def url_helpers end def environment - environment_params = project.repository.branch_exists?(blob.commit_id) ? { ref: blob.commit_id } : { sha: blob.commit_id } + environment_params = project.repository.branch_exists?(commit_id) ? { ref: commit_id } : { sha: commit_id } environment_params[:find_latest] = true ::Environments::EnvironmentsByDeploymentsFinder.new(project, current_user, environment_params).execute.last end @@ -190,12 +190,13 @@ def project blob.repository.project end - def ref_qualified_path + def commit_id # If `ref_type` is present the commit_id will include the ref qualifier e.g. `refs/heads/`. # We only accept/return unqualified refs so we need to remove the qualifier from the `commit_id`. + ExtractsRef.unqualify_ref(blob.commit_id, ref_type) + end - commit_id = ExtractsRef.unqualify_ref(blob.commit_id, ref_type) - + def ref_qualified_path File.join(commit_id, blob.path) end diff --git a/spec/presenters/blob_presenter_spec.rb b/spec/presenters/blob_presenter_spec.rb index e776716bd2d724b8d6ad0bc5fe2d5aef12bec29e..150c7bd5f3edbea0c5aba6c109b018404608c474 100644 --- a/spec/presenters/blob_presenter_spec.rb +++ b/spec/presenters/blob_presenter_spec.rb @@ -7,28 +7,52 @@ let_it_be(:user) { project.first_owner } let(:repository) { project.repository } - let(:blob) { repository.blob_at('HEAD', 'files/ruby/regex.rb') } + let(:blob) { repository.blob_at(ref, path) } + let(:ref) { 'HEAD' } + let(:path) { 'files/ruby/regex.rb' } subject(:presenter) { described_class.new(blob, current_user: user) } describe '#web_url' do - it { expect(presenter.web_url).to eq("http://localhost/#{project.full_path}/-/blob/#{blob.commit_id}/#{blob.path}") } + it { expect(presenter.web_url).to eq("http://localhost/#{project.full_path}/-/blob/#{ref}/#{path}") } end describe '#web_path' do - it { expect(presenter.web_path).to eq("/#{project.full_path}/-/blob/#{blob.commit_id}/#{blob.path}") } + it { expect(presenter.web_path).to eq("/#{project.full_path}/-/blob/#{ref}/#{path}") } end describe '#edit_blob_path' do - it { expect(presenter.edit_blob_path).to eq("/#{project.full_path}/-/edit/#{blob.commit_id}/#{blob.path}") } + it { expect(presenter.edit_blob_path).to eq("/#{project.full_path}/-/edit/#{ref}/#{path}") } end describe '#raw_path' do - it { expect(presenter.raw_path).to eq("/#{project.full_path}/-/raw/#{blob.commit_id}/#{blob.path}") } + it { expect(presenter.raw_path).to eq("/#{project.full_path}/-/raw/#{ref}/#{path}") } end describe '#replace_path' do - it { expect(presenter.replace_path).to eq("/#{project.full_path}/-/update/#{blob.commit_id}/#{blob.path}") } + it { expect(presenter.replace_path).to eq("/#{project.full_path}/-/update/#{ref}/#{path}") } + end + + shared_examples_for '#can_current_user_push_to_branch?' do + let(:branch_exists) { true } + + before do + allow(project.repository).to receive(:branch_exists?).with(blob.commit_id).and_return(branch_exists) + end + + it { expect(presenter.can_current_user_push_to_branch?).to eq(true) } + + context 'current_user is nil' do + let(:user) { nil } + + it { expect(presenter.can_current_user_push_to_branch?).to eq(false) } + end + + context 'branch does not exist' do + let(:branch_exists) { false } + + it { expect(presenter.can_current_user_push_to_branch?).to eq(false) } + end end context 'when blob has ref_type' do @@ -37,46 +61,67 @@ end describe '#web_url' do - it { expect(presenter.web_url).to eq("http://localhost/#{project.full_path}/-/blob/#{blob.commit_id}/#{blob.path}?ref_type=heads") } + it { expect(presenter.web_url).to eq("http://localhost/#{project.full_path}/-/blob/#{ref}/#{path}?ref_type=heads") } end describe '#web_path' do - it { expect(presenter.web_path).to eq("/#{project.full_path}/-/blob/#{blob.commit_id}/#{blob.path}?ref_type=heads") } + it { expect(presenter.web_path).to eq("/#{project.full_path}/-/blob/#{ref}/#{path}?ref_type=heads") } end describe '#edit_blob_path' do - it { expect(presenter.edit_blob_path).to eq("/#{project.full_path}/-/edit/#{blob.commit_id}/#{blob.path}?ref_type=heads") } + it { expect(presenter.edit_blob_path).to eq("/#{project.full_path}/-/edit/#{ref}/#{path}?ref_type=heads") } end describe '#raw_path' do - it { expect(presenter.raw_path).to eq("/#{project.full_path}/-/raw/#{blob.commit_id}/#{blob.path}?ref_type=heads") } + it { expect(presenter.raw_path).to eq("/#{project.full_path}/-/raw/#{ref}/#{path}?ref_type=heads") } end describe '#replace_path' do - it { expect(presenter.replace_path).to eq("/#{project.full_path}/-/update/#{blob.commit_id}/#{blob.path}?ref_type=heads") } + it { expect(presenter.replace_path).to eq("/#{project.full_path}/-/update/#{ref}/#{path}?ref_type=heads") } end + + it_behaves_like '#can_current_user_push_to_branch?' end - describe '#can_current_user_push_to_branch' do - let(:branch_exists) { true } + describe '#can_modify_blob?' do + context 'when blob is store externally' do + before do + allow(blob).to receive(:stored_externally?).and_return(true) + end - before do - allow(project.repository).to receive(:branch_exists?).with(blob.commit_id).and_return(branch_exists) + it { expect(presenter.can_modify_blob?).to be_falsey } end - it { expect(presenter.can_current_user_push_to_branch?).to eq(true) } + context 'when the user cannot edit the tree' do + before do + allow(presenter).to receive(:can_edit_tree?).with(project, ref).and_return(false) + end - context 'current_user is nil' do - let(:user) { nil } + it { expect(presenter.can_modify_blob?).to be_falsey } + end - it { expect(presenter.can_current_user_push_to_branch?).to eq(false) } + context 'when ref is a branch' do + let(:ref) { 'feature' } + + it { expect(presenter.can_modify_blob?).to be_truthy } end + end - context 'branch does not exist' do - let(:branch_exists) { false } + describe '#can_current_user_push_to_branch?' do + context 'when ref is a branch' do + let(:ref) { 'feature' } - it { expect(presenter.can_current_user_push_to_branch?).to eq(false) } + it 'delegates to UserAccess' do + allow_next_instance_of(Gitlab::UserAccess) do |instance| + expect(instance).to receive(:can_push_to_branch?).with(ref).and_call_original + end + expect(presenter.can_current_user_push_to_branch?).to be_truthy + end end + + it_behaves_like '#can_current_user_push_to_branch?' + + it { expect(presenter.can_current_user_push_to_branch?).to be_falsey } end describe '#archived?' do @@ -95,9 +140,10 @@ ) end - let(:blob) { repository.blob_at('main', '.gitlab-ci.yml') } + let(:ref) { 'main' } + let(:path) { '.gitlab-ci.yml' } - it { expect(presenter.pipeline_editor_path).to eq("/#{project.full_path}/-/ci/editor?branch_name=#{blob.commit_id}") } + it { expect(presenter.pipeline_editor_path).to eq("/#{project.full_path}/-/ci/editor?branch_name=#{ref}") } end end @@ -114,7 +160,7 @@ context 'Gitpod enabled for application and user' do describe '#gitpod_blob_url' do - it { expect(presenter.gitpod_blob_url).to eq("#{gitpod_url}##{"http://localhost/#{project.full_path}/-/tree/#{blob.commit_id}/#{blob.path}"}") } + it { expect(presenter.gitpod_blob_url).to eq("#{gitpod_url}##{"http://localhost/#{project.full_path}/-/tree/#{ref}/#{path}"}") } end end @@ -157,7 +203,7 @@ let!(:deployment) { create(:deployment, :success, environment: environment, project: project, sha: blob.commit_id) } before do - allow(project).to receive(:public_path_for_source_path).with(blob.path, blob.commit_id).and_return(blob.path) + allow(project).to receive(:public_path_for_source_path).with(path, blob.commit_id).and_return(path) end describe '#environment_formatted_external_url' do @@ -165,7 +211,7 @@ end describe '#environment_external_url_for_route_map' do - it { expect(presenter.environment_external_url_for_route_map).to eq("#{external_url}/#{blob.path}") } + it { expect(presenter.environment_external_url_for_route_map).to eq("#{external_url}/#{path}") } end describe 'chooses the latest deployed environment for #environment_formatted_external_url and #environment_external_url_for_route_map' do @@ -174,7 +220,7 @@ let!(:another_deployment) { create(:deployment, :success, environment: another_environment, project: project, sha: blob.commit_id) } it { expect(presenter.environment_formatted_external_url).to eq("another.environment") } - it { expect(presenter.environment_external_url_for_route_map).to eq("#{another_external_url}/#{blob.path}") } + it { expect(presenter.environment_external_url_for_route_map).to eq("#{another_external_url}/#{path}") } end end @@ -219,7 +265,7 @@ end describe '#code_navigation_path' do - let(:code_navigation_path) { Gitlab::CodeNavigationPath.new(project, blob.commit_id).full_json_path_for(blob.path) } + let(:code_navigation_path) { Gitlab::CodeNavigationPath.new(project, blob.commit_id).full_json_path_for(path) } it { expect(presenter.code_navigation_path).to eq(code_navigation_path) } end @@ -232,11 +278,11 @@ let(:blob) { Gitlab::Graphql::Representation::TreeEntry.new(super(), repository) } describe '#web_url' do - it { expect(presenter.web_url).to eq("http://localhost/#{project.full_path}/-/blob/#{blob.commit_id}/#{blob.path}") } + it { expect(presenter.web_url).to eq("http://localhost/#{project.full_path}/-/blob/#{ref}/#{path}") } end describe '#web_path' do - it { expect(presenter.web_path).to eq("/#{project.full_path}/-/blob/#{blob.commit_id}/#{blob.path}") } + it { expect(presenter.web_path).to eq("/#{project.full_path}/-/blob/#{ref}/#{path}") } end end