diff options
18 files changed, 390 insertions, 42 deletions
diff --git a/app/assets/stylesheets/framework/lists.scss b/app/assets/stylesheets/framework/lists.scss index 965fcc06518..a88c7906f5d 100644 --- a/app/assets/stylesheets/framework/lists.scss +++ b/app/assets/stylesheets/framework/lists.scss @@ -162,6 +162,10 @@ ul.content-list { margin-right: 0; } } + + .artifacts-btn { + margin-right: 10px; + } } // When dragging a list item diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index 7241949393b..05112571225 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -5,18 +5,18 @@ class Projects::ArtifactsController < Projects::ApplicationController before_action :validate_artifacts! def download - unless artifacts_file.file_storage? - return redirect_to artifacts_file.url + if artifacts_file.file_storage? + send_file artifacts_file.path, disposition: 'attachment' + else + redirect_to artifacts_file.url end - - send_file artifacts_file.path, disposition: 'attachment' end def browse directory = params[:path] ? "#{params[:path]}/" : '' @entry = build.artifacts_metadata_entry(directory) - return render_404 unless @entry.exists? + render_404 unless @entry.exists? end def file @@ -34,14 +34,36 @@ class Projects::ArtifactsController < Projects::ApplicationController redirect_to namespace_project_build_path(project.namespace, project, build) end + def search + if params[:path] + url = namespace_project_build_url(project.namespace, project, build) + + redirect_to "#{url}/artifacts/#{params[:path]}" + else + render_404 + end + end + private def validate_artifacts! - render_404 unless build.artifacts? + render_404 unless build && build.artifacts? end def build - @build ||= project.builds.find_by!(id: params[:build_id]) + @build ||= build_from_id || build_from_ref + end + + def build_from_id + project.builds.find_by(id: params[:build_id]) if params[:build_id] + end + + def build_from_ref + if params[:ref_name] + builds = project.latest_successful_builds_for(params[:ref_name]) + + builds.find_by(name: params[:job]) + end end def artifacts_file diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 08f396210c9..a22caa7d296 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -461,7 +461,7 @@ module Ci def build_attributes_from_config return {} unless pipeline.config_processor - + pipeline.config_processor.build_attributes(name) end end diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 4bd85061240..b096516c627 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -27,6 +27,25 @@ = link_to namespace_project_compare_index_path(@project.namespace, @project, from: @repository.root_ref, to: branch.name), class: 'btn btn-default', method: :post, title: "Compare" do Compare + - pipeline = @project.pipelines.latest_successful_for(branch.name).first + - if pipeline + - artifacts = pipeline.builds.latest.with_artifacts + - if artifacts.any? + .dropdown.inline.artifacts-btn + %a.btn.dropdown-toggle{ 'data-toggle' => 'dropdown' } + = icon('download') + %span.caret + %span.sr-only + Select Archive Format + %ul.dropdown-menu.dropdown-menu-align-right{ role: 'menu' } + %li.dropdown-header Artifacts + - unless pipeline.latest? + = " (not latest, but #{link_to(pipeline.short_sha, namespace_project_pipeline_path(@project.namespace, @project, pipeline))})" + - artifacts.each do |job| + %li + = link_to search_namespace_project_artifacts_path(@project.namespace, @project, branch.name, 'download', job: job.name), rel: 'nofollow' do + %span Download '#{job.name}' + - if can_remove_branch?(@project, branch.name) = link_to namespace_project_branch_path(@project.namespace, @project, branch.name), class: 'btn btn-remove remove-row has-tooltip', title: "Delete branch", method: :delete, data: { confirm: "Deleting the '#{branch.name}' branch cannot be undone. Are you sure?", container: 'body' }, remote: true do = icon("trash-o") diff --git a/app/views/projects/buttons/_download.html.haml b/app/views/projects/buttons/_download.html.haml index 58f43ecb5d5..d135b448dd7 100644 --- a/app/views/projects/buttons/_download.html.haml +++ b/app/views/projects/buttons/_download.html.haml @@ -1,4 +1,27 @@ - unless @project.empty_repo? - if can? current_user, :download_code, @project - = link_to archive_namespace_project_repository_path(@project.namespace, @project, ref: @ref, format: 'zip'), class: 'btn has-tooltip', data: {container: "body"}, rel: 'nofollow', title: "Download ZIP" do - = icon('download') + .dropdown.inline.btn-group + %button.btn{ 'data-toggle' => 'dropdown' } + = icon('download') + = icon('caret-down') + %span.sr-only + Select Archive Format + %ul.dropdown-menu.dropdown-menu-align-right{ role: 'menu' } + %li.dropdown-header Source code + %li + = link_to archive_namespace_project_repository_path(@project.namespace, @project, ref: @ref, format: 'zip'), rel: 'nofollow' do + %span Download zip + %li + = link_to archive_namespace_project_repository_path(@project.namespace, @project, ref: @ref, format: 'tar.gz'), rel: 'nofollow' do + %span Download tar.gz + - pipeline = @project.pipelines.latest_successful_for(@ref).first + - if pipeline + - artifacts = pipeline.builds.latest.with_artifacts + - if artifacts.any? + %li.dropdown-header Artifacts + - unless pipeline.latest? + = " (not latest, but #{link_to(pipeline.short_sha, namespace_project_pipeline_path(@project.namespace, @project, pipeline))})" + - artifacts.each do |job| + %li + = link_to search_namespace_project_artifacts_path(@project.namespace, @project, @ref, 'download', job: job.name), rel: 'nofollow' do + %span Download '#{job.name}' diff --git a/app/views/projects/repositories/_download_archive.html.haml b/app/views/projects/repositories/_download_archive.html.haml index 24658319060..183daebfc3a 100644 --- a/app/views/projects/repositories/_download_archive.html.haml +++ b/app/views/projects/repositories/_download_archive.html.haml @@ -1,16 +1,14 @@ - ref = ref || nil - btn_class = btn_class || '' -- split_button = split_button || false -- if split_button == true - %span.btn-group{class: btn_class} - = link_to archive_namespace_project_repository_path(@project.namespace, @project, ref: ref, format: 'zip'), class: 'btn col-xs-10', rel: 'nofollow' do - %i.fa.fa-download - %span Download zip - %a.col-xs-2.btn.dropdown-toggle{ 'data-toggle' => 'dropdown' } +%span.btn-group{class: btn_class} + .dropdown.inline + %button.btn{ 'data-toggle' => 'dropdown' } + = icon('download') %span.caret %span.sr-only Select Archive Format - %ul.col-xs-10.dropdown-menu.dropdown-menu-align-right{ role: 'menu' } + %ul.dropdown-menu.dropdown-menu-align-right{ role: 'menu' } + %li.dropdown-header Source code %li = link_to archive_namespace_project_repository_path(@project.namespace, @project, ref: ref, format: 'zip'), rel: 'nofollow' do %i.fa.fa-download @@ -27,11 +25,14 @@ = link_to archive_namespace_project_repository_path(@project.namespace, @project, ref: ref, format: 'tar'), rel: 'nofollow' do %i.fa.fa-download %span Download tar -- else - %span.btn-group{class: btn_class} - = link_to archive_namespace_project_repository_path(@project.namespace, @project, ref: ref, format: 'zip'), class: 'btn', rel: 'nofollow' do - %i.fa.fa-download - %span zip - = link_to archive_namespace_project_repository_path(@project.namespace, @project, ref: ref, format: 'tar.gz'), class: 'btn', rel: 'nofollow' do - %i.fa.fa-download - %span tar.gz + - pipeline = @project.pipelines.latest_successful_for(ref).first + - if pipeline + - artifacts = pipeline.builds.latest.with_artifacts + - if artifacts.any? + %li.dropdown-header Artifacts + - unless pipeline.latest? + = " (not latest, but #{link_to(pipeline.short_sha, namespace_project_pipeline_path(@project.namespace, @project, pipeline))})" + - artifacts.each do |job| + %li + = link_to search_namespace_project_artifacts_path(@project.namespace, @project, ref, 'download', job: job.name), rel: 'nofollow' do + %span Download '#{job.name}' diff --git a/app/views/projects/tags/_download.html.haml b/app/views/projects/tags/_download.html.haml index 8a11dbfa9f4..bb2e5224306 100644 --- a/app/views/projects/tags/_download.html.haml +++ b/app/views/projects/tags/_download.html.haml @@ -1,14 +1,25 @@ -%span.btn-group - = link_to archive_namespace_project_repository_path(project.namespace, project, ref: ref, format: 'zip'), class: 'btn btn-default', rel: 'nofollow' do - %span Source code - %a.btn.btn-default.dropdown-toggle{ 'data-toggle' => 'dropdown' } +.dropdown.inline + %a.btn.dropdown-toggle{ 'data-toggle' => 'dropdown' } + = icon('download') %span.caret %span.sr-only Select Archive Format %ul.dropdown-menu.dropdown-menu-align-right{ role: 'menu' } + %li.dropdown-header Source code %li = link_to archive_namespace_project_repository_path(project.namespace, project, ref: ref, format: 'zip'), rel: 'nofollow' do %span Download zip %li = link_to archive_namespace_project_repository_path(project.namespace, project, ref: ref, format: 'tar.gz'), rel: 'nofollow' do %span Download tar.gz + - pipeline = project.pipelines.latest_successful_for(ref).first + - if pipeline + - artifacts = pipeline.builds.latest.with_artifacts + - if artifacts.any? + %li.dropdown-header Artifacts + - unless pipeline.latest? + = " (not latest, but #{link_to(pipeline.short_sha, namespace_project_pipeline_path(project.namespace, project, pipeline))})" + - artifacts.each do |job| + %li + = link_to search_namespace_project_artifacts_path(project.namespace, project, ref, 'download', job: job.name), rel: 'nofollow' do + %span Download '#{job.name}' diff --git a/app/views/projects/tree/show.html.haml b/app/views/projects/tree/show.html.haml index bf5360b4dee..c68f86f1378 100644 --- a/app/views/projects/tree/show.html.haml +++ b/app/views/projects/tree/show.html.haml @@ -11,7 +11,7 @@ .tree-controls = render 'projects/find_file_link' - if can? current_user, :download_code, @project - = render 'projects/repositories/download_archive', ref: @ref, btn_class: 'hidden-xs hidden-sm btn-grouped', split_button: true + = render 'projects/repositories/download_archive', ref: @ref, btn_class: 'hidden-xs hidden-sm btn-grouped' #tree-holder.tree-holder.clearfix .nav-block diff --git a/config/routes.rb b/config/routes.rb index 2f5f32d9e30..256c7dacd59 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -750,6 +750,14 @@ Rails.application.routes.draw do resources :builds, only: [:index, :show], constraints: { id: /\d+/ } do collection do post :cancel_all + + resources :artifacts, only: [] do + collection do + get :search, path: ':ref_name/*path', + format: false, + constraints: { ref_name: /.+/ } # could have / + end + end end member do diff --git a/spec/features/projects/branches/download_buttons_spec.rb b/spec/features/projects/branches/download_buttons_spec.rb new file mode 100644 index 00000000000..d3f53b65699 --- /dev/null +++ b/spec/features/projects/branches/download_buttons_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +feature 'Download buttons in branches page', feature: true do + given(:user) { create(:user) } + given(:role) { :developer } + given(:status) { 'success' } + given(:project) { create(:project) } + given(:pipeline) do + create(:ci_pipeline, project: project, + sha: project.commit.sha, + ref: project.default_branch, + status: status) + end + given!(:build) do + create(:ci_build, :success, :artifacts, + pipeline: pipeline, + status: pipeline.status, + name: 'build') + end + + background do + login_as(user) + project.team << [user, role] + end + + describe 'when checking branches' do + context 'with artifacts' do + before do + visit namespace_project_branches_path(project.namespace, project) + end + + scenario 'shows download artifacts button' do + expect(page).to have_link "Download '#{build.name}'" + end + end + end +end diff --git a/spec/features/builds_spec.rb b/spec/features/projects/builds_spec.rb index 0cfeb2e57d8..0cfeb2e57d8 100644 --- a/spec/features/builds_spec.rb +++ b/spec/features/projects/builds_spec.rb diff --git a/spec/features/projects/files/download_buttons_spec.rb b/spec/features/projects/files/download_buttons_spec.rb new file mode 100644 index 00000000000..60c2ffedce0 --- /dev/null +++ b/spec/features/projects/files/download_buttons_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +feature 'Download buttons in files tree', feature: true do + given(:user) { create(:user) } + given(:role) { :developer } + given(:status) { 'success' } + given(:project) { create(:project) } + given(:pipeline) do + create(:ci_pipeline, project: project, + sha: project.commit.sha, + ref: project.default_branch, + status: status) + end + given!(:build) do + create(:ci_build, :success, :artifacts, + pipeline: pipeline, + status: pipeline.status, + name: 'build') + end + + background do + login_as(user) + project.team << [user, role] + end + + describe 'when files tree' do + context 'with artifacts' do + before do + visit namespace_project_tree_path( + project.namespace, project, project.default_branch) + end + + scenario 'shows download artifacts button' do + expect(page).to have_link "Download '#{build.name}'" + end + end + end +end diff --git a/spec/features/projects/main/download_buttons_spec.rb b/spec/features/projects/main/download_buttons_spec.rb new file mode 100644 index 00000000000..62e56808558 --- /dev/null +++ b/spec/features/projects/main/download_buttons_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +feature 'Download buttons in project main page', feature: true do + given(:user) { create(:user) } + given(:role) { :developer } + given(:status) { 'success' } + given(:project) { create(:project) } + given(:pipeline) do + create(:ci_pipeline, project: project, + sha: project.commit.sha, + ref: project.default_branch, + status: status) + end + given!(:build) do + create(:ci_build, :success, :artifacts, + pipeline: pipeline, + status: pipeline.status, + name: 'build') + end + + background do + login_as(user) + project.team << [user, role] + end + + describe 'when checking project main page' do + context 'with artifacts' do + before do + visit namespace_project_path(project.namespace, project) + end + + scenario 'shows download artifacts button' do + expect(page).to have_link "Download '#{build.name}'" + end + end + end +end diff --git a/spec/features/projects/tags/download_buttons_spec.rb b/spec/features/projects/tags/download_buttons_spec.rb new file mode 100644 index 00000000000..d4c4cfe9c99 --- /dev/null +++ b/spec/features/projects/tags/download_buttons_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +feature 'Download buttons in tags page', feature: true do + given(:user) { create(:user) } + given(:role) { :developer } + given(:status) { 'success' } + given(:tag) { 'v1.0.0' } + given(:project) { create(:project) } + given(:pipeline) do + create(:ci_pipeline, project: project, + sha: project.commit.sha, + ref: tag, + status: status) + end + given!(:build) do + create(:ci_build, :success, :artifacts, + pipeline: pipeline, + status: pipeline.status, + name: 'build') + end + + background do + login_as(user) + project.team << [user, role] + end + + describe 'when checking tags' do + context 'with artifacts' do + before do + visit namespace_project_tags_path(project.namespace, project) + end + + scenario 'shows download artifacts button' do + expect(page).to have_link "Download '#{build.name}'" + end + end + end +end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 9ecc9aac84b..1812c5b9567 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -901,15 +901,17 @@ describe Ci::Build, models: true do before { build.run! } it 'returns false' do - expect(build.retryable?).to be false + expect(build).not_to be_retryable end end context 'when build is finished' do - before { build.success! } + before do + build.success! + end it 'returns true' do - expect(build.retryable?).to be true + expect(build).to be_retryable end end end diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index 966d302dfd3..4da5e211727 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -15,7 +15,9 @@ describe API::API, api: true do describe 'GET /projects/:id/builds ' do let(:query) { '' } - before { get api("/projects/#{project.id}/builds?#{query}", api_user) } + before do + get api("/projects/#{project.id}/builds?#{query}", api_user) + end context 'authorized user' do it 'returns project builds' do @@ -122,7 +124,9 @@ describe API::API, api: true do end describe 'GET /projects/:id/builds/:build_id' do - before { get api("/projects/#{project.id}/builds/#{build.id}", api_user) } + before do + get api("/projects/#{project.id}/builds/#{build.id}", api_user) + end context 'authorized user' do it 'returns specific build data' do @@ -141,7 +145,9 @@ describe API::API, api: true do end describe 'GET /projects/:id/builds/:build_id/artifacts' do - before { get api("/projects/#{project.id}/builds/#{build.id}/artifacts", api_user) } + before do + get api("/projects/#{project.id}/builds/#{build.id}/artifacts", api_user) + end context 'build with artifacts' do let(:build) { create(:ci_build, :artifacts, pipeline: pipeline) } @@ -288,7 +294,9 @@ describe API::API, api: true do end describe 'POST /projects/:id/builds/:build_id/cancel' do - before { post api("/projects/#{project.id}/builds/#{build.id}/cancel", api_user) } + before do + post api("/projects/#{project.id}/builds/#{build.id}/cancel", api_user) + end context 'authorized user' do context 'user with :update_build persmission' do @@ -319,7 +327,9 @@ describe API::API, api: true do describe 'POST /projects/:id/builds/:build_id/retry' do let(:build) { create(:ci_build, :canceled, pipeline: pipeline) } - before { post api("/projects/#{project.id}/builds/#{build.id}/retry", api_user) } + before do + post api("/projects/#{project.id}/builds/#{build.id}/retry", api_user) + end context 'authorized user' do context 'user with :update_build permission' do diff --git a/spec/requests/projects/artifacts_controller_spec.rb b/spec/requests/projects/artifacts_controller_spec.rb new file mode 100644 index 00000000000..b823676d9e1 --- /dev/null +++ b/spec/requests/projects/artifacts_controller_spec.rb @@ -0,0 +1,98 @@ +require 'spec_helper' + +describe Projects::ArtifactsController do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:pipeline) do + create(:ci_pipeline, + project: project, + sha: project.commit.sha, + ref: project.default_branch) + end + let(:build) { create(:ci_build, :success, :artifacts, pipeline: pipeline) } + + describe 'GET /:project/builds/artifacts/:ref_name/browse?job=name' do + before do + project.team << [user, :developer] + end + + before do + login_as(user) + end + + def path_from_ref( + ref = pipeline.ref, job = build.name, path = 'browse') + search_namespace_project_artifacts_path( + project.namespace, + project, + ref, + path, + job: job) + end + + context 'cannot find the build' do + shared_examples 'not found' do + it { expect(response).to have_http_status(:not_found) } + end + + context 'has no such ref' do + before do + get path_from_ref('TAIL', build.name) + end + + it_behaves_like 'not found' + end + + context 'has no such build' do + before do + get path_from_ref(pipeline.ref, 'NOBUILD') + end + + it_behaves_like 'not found' + end + + context 'has no path' do + before do + get path_from_ref(pipeline.sha, build.name, '') + end + + it_behaves_like 'not found' + end + end + + context 'found the build and redirect' do + shared_examples 'redirect to the build' do + it 'redirects' do + path = browse_namespace_project_build_artifacts_path( + project.namespace, + project, + build) + + expect(response).to redirect_to(path) + end + end + + context 'with regular branch' do + before do + pipeline.update(ref: 'master', + sha: project.commit('master').sha) + + get path_from_ref('master') + end + + it_behaves_like 'redirect to the build' + end + + context 'with branch name containing slash' do + before do + pipeline.update(ref: 'improve/awesome', + sha: project.commit('improve/awesome').sha) + + get path_from_ref('improve/awesome') + end + + it_behaves_like 'redirect to the build' + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2e2aa7c4fc0..97294cfc478 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -26,9 +26,9 @@ RSpec.configure do |config| config.verbose_retry = true config.display_try_failure_messages = true - config.include Devise::TestHelpers, type: :controller - config.include LoginHelpers, type: :feature - config.include LoginHelpers, type: :request + config.include Devise::TestHelpers, type: :controller + config.include Warden::Test::Helpers, type: :request + config.include LoginHelpers, type: :feature config.include StubConfiguration config.include EmailHelpers config.include TestEnv |