Merge branch 'artifacts-from-ref-and-build-name' into 'master'

Add a download buttons for Build Artifacts

## What does this MR do?
This MR adds a download buttons for build artifacts of latest succesful pipeline in:
- dashboard of project,
- branches and tags views,
- and tree viewer

Implement #4255

## What are the relevant issue numbers?

Closes #4255, Closes #14419

## Screenshots

### Project main

![](/uploads/29ee2154a214416059a875f2715d4fa3/Screen_Shot_2016-08-24_at_8.00.31_PM.png)

### Branches

![](/uploads/9220c593288370986fbc1d42a1425ef7/Screen_Shot_2016-08-24_at_8.02.01_PM.png)

### Tags

![](/uploads/a843e8103221fea475a0cf9d62a1999d/Screen_Shot_2016-08-24_at_8.03.32_PM.png)

### Source Tree

![](/uploads/63cd3c8c91b6f427c166dc90d8e3c059/Screen_Shot_2016-08-24_at_8.04.56_PM.png)

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] Download buttons
- [x] Models
- [x] Routes
- [x] Projects::ArtifactsController
- [x] API
- Tests
  - Rails
      - [x] Project#builds_for
      - [x] branch name with slashes
      - [x] only success builds
      - [x] only latest builds
      - [x] feature tests for download buttons
  - API
      - [x] branch name with slashes
      - [x] only success builds
      - [x] only latest builds
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !5142

14 files changed, 363 insertions, 28 deletions

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..04058300570
--- /dev/null
+++ b/spec/features/projects/branches/download_buttons_spec.rb
@@ -0,0 +1,40 @@
+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('binary-encoding').sha,
+           ref: 'binary-encoding', # make sure the branch is in the 1st page!
+           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..be5cebcd7c9
--- /dev/null
+++ b/spec/features/projects/files/download_buttons_spec.rb
@@ -0,0 +1,41 @@
+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..b26c0ea7a14
--- /dev/null
+++ b/spec/features/projects/main/download_buttons_spec.rb
@@ -0,0 +1,40 @@
+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..6e0022c179f
--- /dev/null
+++ b/spec/features/projects/tags/download_buttons_spec.rb
@@ -0,0 +1,41 @@
+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(tag).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 ee2c3d04984..c45c2635cf4 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -948,15 +948,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/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 721b20e0cb2..598df576001 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -195,6 +195,36 @@ describe Ci::Pipeline, models: true do
     end
   end
 
+  context 'with non-empty project' do
+    let(:project) { create(:project) }
+
+    let(:pipeline) do
+      create(:ci_pipeline,
+             project: project,
+             ref: project.default_branch,
+             sha: project.commit.sha)
+    end
+
+    describe '#latest?' do
+      context 'with latest sha' do
+        it 'returns true' do
+          expect(pipeline).to be_latest
+        end
+      end
+
+      context 'with not latest sha' do
+        before do
+          pipeline.update(
+            sha: project.commit("#{project.default_branch}~1").sha)
+        end
+
+        it 'returns false' do
+          expect(pipeline).not_to be_latest
+        end
+      end
+    end
+  end
+
   describe '#manual_actions' do
     subject { pipeline.manual_actions }
 
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 901b7bad007..5bf3b8e609e 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -477,8 +477,8 @@ describe MergeRequest, models: true do
 
         allow(subject).to receive(:diff_head_sha).and_return('123abc')
 
-        expect(subject.source_project).to receive(:pipeline).
-          with('123abc', 'master').
+        expect(subject.source_project).to receive(:pipeline_for).
+          with('master', '123abc').
           and_return(pipeline)
 
         expect(subject.pipeline).to eq(pipeline)
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 3b637b0defc..dd309ea1b68 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -697,23 +697,37 @@ describe Project, models: true do
     end
   end
 
-  describe '#pipeline' do
-    let(:project) { create :project }
-    let(:pipeline) { create :ci_pipeline, project: project, ref: 'master' }
-
-    subject { project.pipeline(pipeline.sha, 'master') }
+  describe '#pipeline_for' do
+    let(:project) { create(:project) }
+    let!(:pipeline) { create_pipeline }
 
-    it { is_expected.to eq(pipeline) }
+    shared_examples 'giving the correct pipeline' do
+      it { is_expected.to eq(pipeline) }
 
-    context 'return latest' do
-      let(:pipeline2) { create :ci_pipeline, project: project, ref: 'master' }
+      context 'return latest' do
+        let!(:pipeline2) { create_pipeline }
 
-      before do
-        pipeline
-        pipeline2
+        it { is_expected.to eq(pipeline2) }
       end
+    end
+
+    context 'with explicit sha' do
+      subject { project.pipeline_for('master', pipeline.sha) }
+
+      it_behaves_like 'giving the correct pipeline'
+    end
+
+    context 'with implicit sha' do
+      subject { project.pipeline_for('master') }
+
+      it_behaves_like 'giving the correct pipeline'
+    end
 
-      it { is_expected.to eq(pipeline2) }
+    def create_pipeline
+      create(:ci_pipeline,
+             project: project,
+             ref: 'master',
+             sha: project.commit('master').sha)
     end
   end
 
diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb
index 9a17a705b1e..ee0b61e2ca4 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) }
@@ -292,7 +298,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
@@ -323,7 +331,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/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index 7ca75d77673..5b3dc60aba2 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -95,7 +95,7 @@ describe API::API, api: true  do
       end
 
       it "returns status for CI" do
-        pipeline = project.ensure_pipeline(project.repository.commit.sha, 'master')
+        pipeline = project.ensure_pipeline('master', project.repository.commit.sha)
         pipeline.update(status: 'success')
 
         get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user)
@@ -105,7 +105,7 @@ describe API::API, api: true  do
       end
 
       it "returns status for CI when pipeline is created" do
-        project.ensure_pipeline(project.repository.commit.sha, 'master')
+        project.ensure_pipeline('master', project.repository.commit.sha)
 
         get api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}", user)
 
diff --git a/spec/requests/projects/artifacts_controller_spec.rb b/spec/requests/projects/artifacts_controller_spec.rb
new file mode 100644
index 00000000000..e02f0eacc93
--- /dev/null
+++ b/spec/requests/projects/artifacts_controller_spec.rb
@@ -0,0 +1,117 @@
+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,
+            status: 'success')
+  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]
+
+      login_as(user)
+    end
+
+    def path_from_ref(
+      ref = pipeline.ref, job = build.name, path = 'browse')
+      latest_succeeded_namespace_project_artifacts_path(
+        project.namespace,
+        project,
+        [ref, path].join('/'),
+        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
+
+      context 'with branch name and path containing slashes' do
+        before do
+          pipeline.update(ref: 'improve/awesome',
+                          sha: project.commit('improve/awesome').sha)
+
+          get path_from_ref('improve/awesome', build.name, 'file/README.md')
+        end
+
+        it 'redirects' do
+          path = file_namespace_project_build_artifacts_path(
+            project.namespace,
+            project,
+            build,
+            'README.md')
+
+          expect(response).to redirect_to(path)
+        end
+      end
+    end
+  end
+end
diff --git a/spec/services/ci/image_for_build_service_spec.rb b/spec/services/ci/image_for_build_service_spec.rb
index c931c3e4829..b3e0a7b9b58 100644
--- a/spec/services/ci/image_for_build_service_spec.rb
+++ b/spec/services/ci/image_for_build_service_spec.rb
@@ -5,7 +5,7 @@ module Ci
     let(:service) { ImageForBuildService.new }
     let(:project) { FactoryGirl.create(:empty_project) }
     let(:commit_sha) { '01234567890123456789' }
-    let(:pipeline) { project.ensure_pipeline(commit_sha, 'master') }
+    let(:pipeline) { project.ensure_pipeline('master', commit_sha) }
     let(:build) { FactoryGirl.create(:ci_build, pipeline: pipeline) }
 
     describe '#execute' do
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index c144cd85487..02b2b3ca101 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