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

CHANGELOG

@@ -25,6 +25,7 @@ v 8.12.0 (unreleased)
   - Add horizontal scrolling to all sub-navs on mobile viewports (ClemMakesApps)
   - Fix markdown help references (ClemMakesApps)
   - Added tests for diff notes
+  - Add a button to download latest successful artifacts for branches and tags !5142
   - Add delimiter to project stars and forks count (ClemMakesApps)
   - Fix badge count alignment (ClemMakesApps)
   - Fix branch title trailing space on hover (ClemMakesApps)

app/assets/stylesheets/framework/dropdowns.scss

@@ -195,6 +195,12 @@
   .separator + .dropdown-header {
     padding-top: 2px;
   }
+
+  .unclickable {
+    cursor: not-allowed;
+    padding: 5px 8px;
+    color: $dropdown-header-color;
+  }
 }
 
 .dropdown-menu-large {

app/controllers/projects/artifacts_controller.rb

 class Projects::ArtifactsController < Projects::ApplicationController
+  include ExtractsPath
+
   layout 'project'
   before_action :authorize_read_build!
   before_action :authorize_update_build!, only: [:keep]
+  before_action :extract_ref_name_and_path
   before_action :validate_artifacts!
 
   def download
-    unless artifacts_file.file_storage?
-      return redirect_to artifacts_file.url
-    end
-
+    if artifacts_file.file_storage?
       send_file artifacts_file.path, disposition: 'attachment'
+    else
+      redirect_to artifacts_file.url
+    end
   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 +37,41 @@ class Projects::ArtifactsController < Projects::ApplicationController
     redirect_to namespace_project_build_path(project.namespace, project, build)
   end
 
+  def latest_succeeded
+    target_path = artifacts_action_path(@path, project, build)
+
+    if target_path
+      redirect_to(target_path)
+    else
+      render_404
+    end
+  end
+
   private
 
+  def extract_ref_name_and_path
+    return unless params[:ref_name_and_path]
+
+    @ref_name, @path = extract_ref(params[:ref_name_and_path])
+  end
+
   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
+    return unless @ref_name
+
+    builds = project.latest_successful_builds_for(@ref_name)
+    builds.find_by(name: params[:job])
   end
 
   def artifacts_file

app/helpers/ci_status_helper.rb

@@ -25,6 +25,11 @@ module CiStatusHelper
     end
   end
 
+  def ci_status_for_statuseable(subject)
+    status = subject.try(:status) || 'not found'
+    status.humanize
+  end
+
   def ci_icon_for_status(status)
     icon_name =
       case status

app/helpers/gitlab_routing_helper.rb

@@ -149,4 +149,20 @@ module GitlabRoutingHelper
   def resend_invite_group_member_path(group_member, *args)
     resend_invite_group_group_member_path(group_member.source, group_member)
   end
+
+  # Artifacts
+
+  def artifacts_action_path(path, project, build)
+    action, path_params = path.split('/', 2)
+    args = [project.namespace, project, build, path_params]
+
+    case action
+    when 'download'
+      download_namespace_project_build_artifacts_path(*args)
+    when 'browse'
+      browse_namespace_project_build_artifacts_path(*args)
+    when 'file'
+      file_namespace_project_build_artifacts_path(*args)
+    end
+  end
 end

app/models/ci/pipeline.rb

@@ -65,8 +65,8 @@ module Ci
     end
 
     # ref can't be HEAD or SHA, can only be branch/tag name
-    scope :latest_successful_for, ->(ref = default_branch) do
-      where(ref: ref).success.order(id: :desc).limit(1)
+    def self.latest_successful_for(ref)
+      where(ref: ref).order(id: :desc).success.first
     end
 
     def self.truncate_sha(sha)

app/models/merge_request.rb

@@ -729,7 +729,9 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def pipeline
-    @pipeline ||= source_project.pipeline(diff_head_sha, source_branch) if diff_head_sha && source_project
+    return unless diff_head_sha && source_project
+
+    @pipeline ||= source_project.pipeline_for(source_branch, diff_head_sha)
   end
 
   def all_pipelines

app/models/project.rb

@@ -436,7 +436,7 @@ class Project < ActiveRecord::Base
 
   # ref can't be HEAD, can only be branch/tag name or SHA
   def latest_successful_builds_for(ref = default_branch)
-    latest_pipeline = pipelines.latest_successful_for(ref).first
+    latest_pipeline = pipelines.latest_successful_for(ref)
 
     if latest_pipeline
       latest_pipeline.builds.latest.with_artifacts
@@ -1100,12 +1100,17 @@ class Project < ActiveRecord::Base
     !namespace.share_with_group_lock
   end
 
-  def pipeline(sha, ref)
+  def pipeline_for(ref, sha = nil)
+    sha ||= commit(ref).try(:sha)
+
+    return unless sha
+
     pipelines.order(id: :desc).find_by(sha: sha, ref: ref)
   end
 
-  def ensure_pipeline(sha, ref, current_user = nil)
-    pipeline(sha, ref) || pipelines.create(sha: sha, ref: ref, user: current_user)
+  def ensure_pipeline(ref, sha, current_user = nil)
+    pipeline_for(ref, sha) ||
+      pipelines.create(sha: sha, ref: ref, user: current_user)
   end
 
   def enable_ci

app/views/projects/branches/_branch.html.haml

@@ -27,6 +27,8 @@
         = 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
 
+      = render 'projects/buttons/download', project: @project, ref: branch.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")

app/views/projects/buttons/_download.html.haml

-- 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
+- if !project.empty_repo? && can?(current_user, :download_code, project)
+  %span.btn-group{class: 'hidden-xs hidden-sm btn-grouped'}
+    .dropdown.inline
+      %button.btn{ '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
+            %i.fa.fa-download
+            %span Download zip
+        %li
+          = link_to archive_namespace_project_repository_path(project.namespace, project, ref: ref, format: 'tar.gz'), rel: 'nofollow' do
+            %i.fa.fa-download
+            %span Download tar.gz
+        %li
+          = link_to archive_namespace_project_repository_path(project.namespace, project, ref: ref, format: 'tar.bz2'), rel: 'nofollow' do
+            %i.fa.fa-download
+            %span Download tar.bz2
+        %li
+          = link_to archive_namespace_project_repository_path(project.namespace, project, ref: ref, format: 'tar'), rel: 'nofollow' do
+            %i.fa.fa-download
+            %span Download tar
+
+        - pipeline = project.pipelines.latest_successful_for(ref)
+        - if pipeline
+          - artifacts = pipeline.builds.latest.with_artifacts
+          - if artifacts.any?
+            %li.dropdown-header Artifacts
+            - unless pipeline.latest?
+              - latest_pipeline = project.pipeline_for(ref)
+              %li
+                .unclickable= ci_status_for_statuseable(latest_pipeline)
+              %li.dropdown-header Previous Artifacts
+            - artifacts.each do |job|
+              %li
+                = link_to latest_succeeded_namespace_project_artifacts_path(project.namespace, project, ref, 'download', job: job.name), rel: 'nofollow' do
+                  %i.fa.fa-download
+                  %span Download '#{job.name}'

app/views/projects/issues/_related_branches.html.haml

@@ -5,7 +5,7 @@
     - @related_branches.each do |branch|
       %li
         - target = @project.repository.find_branch(branch).target
-        - pipeline = @project.pipeline(target.sha, branch) if target
+        - pipeline = @project.pipeline_for(branch, target.sha) if target
         - if pipeline
           %span.related-branch-ci-status
             = render_pipeline_status(pipeline)

app/views/projects/repositories/_download_archive.html.haml

-- 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.caret
-      %span.sr-only
-        Select Archive Format
-    %ul.col-xs-10.dropdown-menu.dropdown-menu-align-right{ role: 'menu' }
-      %li
-        = link_to archive_namespace_project_repository_path(@project.namespace, @project, ref: ref, format: 'zip'), rel: 'nofollow' do
-          %i.fa.fa-download
-          %span Download zip
-      %li
-        = link_to archive_namespace_project_repository_path(@project.namespace, @project, ref: ref, format: 'tar.gz'), rel: 'nofollow' do
-          %i.fa.fa-download
-          %span Download tar.gz
-      %li
-        = link_to archive_namespace_project_repository_path(@project.namespace, @project, ref: ref, format: 'tar.bz2'), rel: 'nofollow' do
-          %i.fa.fa-download
-          %span Download tar.bz2
-      %li
-        = 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

app/views/projects/show.html.haml

@@ -72,7 +72,7 @@
           = render "projects/buttons/koding"
 
         .btn-group.project-repo-btn-group
-          = render "projects/buttons/download"
+          = render 'projects/buttons/download', project: @project, ref: @ref
           = render 'projects/buttons/dropdown'
 
         = render 'shared/notifications/button', notification_setting: @notification_setting

app/views/projects/tags/_download.html.haml

-%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' }
-    %span.caret
-    %span.sr-only
-      Select Archive Format
-  %ul.dropdown-menu.dropdown-menu-align-right{ role: 'menu' }
-    %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

app/views/projects/tags/_tag.html.haml

@@ -11,8 +11,7 @@
       = strip_gpg_signature(tag.message)
 
     .controls
-      - if can?(current_user, :download_code, @project)
-        = render 'projects/tags/download', ref: tag.name, project: @project
+      = render 'projects/buttons/download', project: @project, ref: tag.name
 
       - if can?(current_user, :push_code, @project)
         = link_to edit_namespace_project_tag_release_path(@project.namespace, @project, tag.name), class: 'btn has-tooltip', title: "Edit release notes", data: { container: "body" } do

app/views/projects/tags/show.html.haml

@@ -12,8 +12,7 @@
         = icon('files-o')
       = link_to namespace_project_commits_path(@project.namespace, @project, @tag.name), class: 'btn has-tooltip', title: 'Browse commits' do
         = icon('history')
-      - if can? current_user, :download_code, @project
-        = render 'projects/tags/download', ref: @tag.name, project: @project
+      = render 'projects/buttons/download', project: @project, ref: @tag.name
       - if can?(current_user, :admin_project, @project)
         .pull-right
           = link_to namespace_project_tag_path(@project.namespace, @project, @tag.name), class: 'btn btn-remove remove-row grouped has-tooltip', title: "Delete tag", method: :delete, data: { confirm: "Deleting the '#{@tag.name}' tag cannot be undone. Are you sure?" } do

app/views/projects/tree/show.html.haml

@@ -10,8 +10,7 @@
 %div{ class: container_class }
   .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/buttons/download', project: @project, ref: @ref
 
   #tree-holder.tree-holder.clearfix
     .nav-block

config/routes.rb

@@ -782,6 +782,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 :latest_succeeded,
+                  path: '*ref_name_and_path',
+                  format: false
+              end
+            end
           end
 
           member do

lib/api/commit_statuses.rb

@@ -64,7 +64,7 @@ module API
           ref = branches.first
         end
 
-        pipeline = @project.ensure_pipeline(commit.sha, ref, current_user)
+        pipeline = @project.ensure_pipeline(ref, commit.sha, current_user)
 
         name = params[:name] || params[:context]
         status = GenericCommitStatus.running_or_pending.find_by(pipeline: pipeline, name: name, ref: params[:ref])

lib/gitlab/badge/coverage/report.rb

@@ -12,9 +12,7 @@ module Gitlab
           @ref = ref
           @job = job
 
-          @pipeline = @project.pipelines
-            .latest_successful_for(@ref)
-            .first
+          @pipeline = @project.pipelines.latest_successful_for(@ref)
         end
 
         def entity

spec/features/projects/branches/download_buttons_spec.rb

+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

spec/features/projects/files/download_buttons_spec.rb

+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

spec/features/projects/main/download_buttons_spec.rb

+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

spec/features/projects/tags/download_buttons_spec.rb

+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

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

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 }
 

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)

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 }
 
+    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' }
+        let!(:pipeline2) { create_pipeline }
 
-      before do
-        pipeline
-        pipeline2
+        it { is_expected.to eq(pipeline2) }
+      end
     end
 
-      it { is_expected.to eq(pipeline2) }
+    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
+
+    def create_pipeline
+      create(:ci_pipeline,
+             project: project,
+             ref: 'master',
+             sha: project.commit('master').sha)
     end
   end
 

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

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)
 

spec/requests/projects/artifacts_controller_spec.rb

+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

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

spec/spec_helper.rb

@@ -27,8 +27,8 @@ RSpec.configure do |config|
   config.display_try_failure_messages = true
 
   config.include Devise::TestHelpers,   type: :controller
+  config.include Warden::Test::Helpers, type: :request
   config.include LoginHelpers,          type: :feature
-  config.include LoginHelpers,        type: :request
   config.include StubConfiguration
   config.include EmailHelpers
   config.include TestEnv