Merge branch 'dm-artifact-browser-header' into 'master'

Add breadcrumb, build header and pipelines submenu to artifacts browser

See merge request !11050

12 files changed, 73 insertions, 42 deletions

diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb
index 59222637961..a13588b4218 100644
--- a/app/controllers/projects/artifacts_controller.rb
+++ b/app/controllers/projects/artifacts_controller.rb
@@ -16,7 +16,8 @@ class Projects::ArtifactsController < Projects::ApplicationController
   end
 
   def browse
-    directory = params[:path] ? "#{params[:path]}/" : ''
+    @path = params[:path]
+    directory = @path ? "#{@path}/" : ''
     @entry = build.artifacts_metadata_entry(directory)
 
     render_404 unless @entry.exists?
@@ -60,7 +61,10 @@ class Projects::ArtifactsController < Projects::ApplicationController
   end
 
   def build
-    @build ||= build_from_id || build_from_ref
+    @build ||= begin
+      build = build_from_id || build_from_ref
+      build&.present(current_user: current_user)
+    end
   end
 
   def build_from_id
diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb
index f7b5a5f4dfc..a91e3da309c 100644
--- a/app/helpers/tree_helper.rb
+++ b/app/helpers/tree_helper.rb
@@ -76,7 +76,7 @@ module TreeHelper
     "A new branch will be created in your fork and a new merge request will be started."
   end
 
-  def tree_breadcrumbs(tree, max_links = 2)
+  def path_breadcrumbs(max_links = 6)
     if @path.present?
       part_path = ""
       parts = @path.split('/')
@@ -88,7 +88,7 @@ module TreeHelper
         part_path = part if part_path.empty?
 
         next if parts.count > max_links && !parts.last(2).include?(part)
-        yield(part, tree_join(@ref, part_path))
+        yield(part, part_path)
       end
     end
   end
diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml
index 8ab9747efc5..cdcac7e4264 100644
--- a/app/views/layouts/nav/_project.html.haml
+++ b/app/views/layouts/nav/_project.html.haml
@@ -38,7 +38,7 @@
             %span.badge.count.merge_counter= number_with_delimiter(MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened.count)
 
     - if project_nav_tab? :pipelines
-      = nav_link(controller: [:pipelines, :builds, :environments]) do
+      = nav_link(controller: [:pipelines, :builds, :environments, :artifacts]) do
         = link_to project_pipelines_path(@project), title: 'Pipelines', class: 'shortcuts-pipelines' do
           %span
             Pipelines
diff --git a/app/views/projects/artifacts/_tree_directory.html.haml b/app/views/projects/artifacts/_tree_directory.html.haml
index 9e49c93388a..34d5c3b7285 100644
--- a/app/views/projects/artifacts/_tree_directory.html.haml
+++ b/app/views/projects/artifacts/_tree_directory.html.haml
@@ -3,6 +3,6 @@
 %tr.tree-item{ 'data-link' => path_to_directory }
   %td.tree-item-file-name
     = tree_icon('folder', '755', directory.name)
-    %span.str-truncated
-      = link_to directory.name, path_to_directory
+    = link_to path_to_directory do
+      %span.str-truncated= directory.name
   %td
diff --git a/app/views/projects/artifacts/browse.html.haml b/app/views/projects/artifacts/browse.html.haml
index de8c173f26f..9fbb30f7c7c 100644
--- a/app/views/projects/artifacts/browse.html.haml
+++ b/app/views/projects/artifacts/browse.html.haml
@@ -1,13 +1,23 @@
-- page_title 'Artifacts', "#{@build.name} (##{@build.id})", 'Jobs'
+- page_title @path.presence, 'Artifacts', "#{@build.name} (##{@build.id})", 'Jobs'
+= render "projects/pipelines/head"
 
-.top-block.row-content-block.clearfix
-  .pull-right
-    = link_to download_namespace_project_build_artifacts_path(@project.namespace, @project, @build),
-      rel: 'nofollow', download: '', class: 'btn btn-default download' do
-      = icon('download')
-      Download artifacts archive
+= render "projects/builds/header", show_controls: false
 
 .tree-holder
+  .nav-block
+    .tree-controls
+      = link_to download_namespace_project_build_artifacts_path(@project.namespace, @project, @build),
+        rel: 'nofollow', download: '', class: 'btn btn-default download' do
+        = icon('download')
+        Download artifacts archive
+
+    %ul.breadcrumb.repo-breadcrumb
+      %li
+        = link_to 'Artifacts', browse_namespace_project_build_artifacts_path(@project.namespace, @project, @build)
+      - path_breadcrumbs do |title, path|
+        %li
+          = link_to truncate(title, length: 40), browse_namespace_project_build_artifacts_path(@project.namespace, @project, @build, path)
+
   .tree-content-holder
     %table.table.tree-table
       %thead
diff --git a/app/views/projects/blob/_blob.html.haml b/app/views/projects/blob/_blob.html.haml
index 3f12d64d044..f04df441ccb 100644
--- a/app/views/projects/blob/_blob.html.haml
+++ b/app/views/projects/blob/_blob.html.haml
@@ -6,17 +6,14 @@
     %li
       = link_to namespace_project_tree_path(@project.namespace, @project, @ref) do
         = @project.path
-    - tree_breadcrumbs(@tree, 6) do |title, path|
+    - path_breadcrumbs do |title, path|
+      - title = truncate(title, length: 40)
       %li
-        - if path
-          - if path.end_with?(@path)
-            = link_to namespace_project_blob_path(@project.namespace, @project, path) do
-              %strong
-                = truncate(title, length: 40)
-          - else
-            = link_to truncate(title, length: 40), namespace_project_tree_path(@project.namespace, @project, path)
+        - if path == @path
+          = link_to namespace_project_blob_path(@project.namespace, @project, tree_join(@ref, path)) do
+            %strong= title
         - else
-          = link_to title, '#'
+          = link_to title, namespace_project_tree_path(@project.namespace, @project, tree_join(@ref, path))
 
 %ul.blob-commit-info.hidden-xs
   - blob_commit = @repository.last_commit_for_path(@commit.id, blob.path)
@@ -25,5 +22,4 @@
 #blob-content-holder.blob-content-holder
   %article.file-holder
     = render "projects/blob/header", blob: blob
-
     = render 'projects/blob/content', blob: blob
diff --git a/app/views/projects/builds/_header.html.haml b/app/views/projects/builds/_header.html.haml
index 104db85809c..a0f8f105d9a 100644
--- a/app/views/projects/builds/_header.html.haml
+++ b/app/views/projects/builds/_header.html.haml
@@ -1,10 +1,13 @@
+- show_controls = local_assigns.fetch(:show_controls, true)
 - pipeline = @build.pipeline
 
 .content-block.build-header.top-area
   .header-content
     = render 'ci/status/badge', status: @build.detailed_status(current_user), link: false, title: @build.status_title
-    Job
-    %strong.js-build-id ##{@build.id}
+    %strong
+      Job
+      = link_to namespace_project_build_path(@project.namespace, @project, @build), class: 'js-build-id' do
+        \##{@build.id}
     in pipeline
     = link_to pipeline_path(pipeline) do
       %strong ##{pipeline.id}
@@ -15,13 +18,16 @@
     = link_to namespace_project_commits_path(@project.namespace, @project, @build.ref) do
       %code
         = @build.ref
-    - if @build.user
-      = render "user"
+
+    = render "projects/builds/user" if @build.user
+
     = time_ago_with_tooltip(@build.created_at)
-  .nav-controls
-    - if can?(current_user, :create_issue, @project) && @build.failed?
-      = link_to "New issue", new_namespace_project_issue_path(@project.namespace, @project, issue: build_failed_issue_options), class: 'btn btn-new btn-inverted'
-    - if can?(current_user, :update_build, @build) && @build.retryable?
-      = link_to "Retry job", retry_namespace_project_build_path(@project.namespace, @project, @build), class: 'btn btn-inverted-secondary', method: :post
-  %button.btn.btn-default.pull-right.visible-xs-block.visible-sm-block.build-gutter-toggle.js-sidebar-build-toggle{ role: "button", type: "button" }
-    = icon('angle-double-left')
+
+  - if show_controls
+    .nav-controls
+      - if can?(current_user, :create_issue, @project) && @build.failed?
+        = link_to "New issue", new_namespace_project_issue_path(@project.namespace, @project, issue: build_failed_issue_options), class: 'btn btn-new btn-inverted'
+      - if can?(current_user, :update_build, @build) && @build.retryable?
+        = link_to "Retry job", retry_namespace_project_build_path(@project.namespace, @project, @build), class: 'btn btn-inverted-secondary', method: :post
+    %button.btn.btn-default.pull-right.visible-xs-block.visible-sm-block.build-gutter-toggle.js-sidebar-build-toggle{ role: "button", type: "button" }
+      = icon('angle-double-left')
diff --git a/app/views/projects/pipelines/_head.html.haml b/app/views/projects/pipelines/_head.html.haml
index bc57f7f1c46..b0dac9de1c6 100644
--- a/app/views/projects/pipelines/_head.html.haml
+++ b/app/views/projects/pipelines/_head.html.haml
@@ -4,13 +4,13 @@
     .nav-links.sub-nav.scrolling-tabs
       %ul{ class: (container_class) }
         - if project_nav_tab? :pipelines
-          = nav_link(path: 'pipelines#index', controller: :pipelines) do
+          = nav_link(path: ['pipelines#index', 'pipelines#show']) do
             = link_to project_pipelines_path(@project), title: 'Pipelines', class: 'shortcuts-pipelines' do
               %span
                 Pipelines
 
         - if project_nav_tab? :builds
-          = nav_link(controller: :builds) do
+          = nav_link(controller: [:builds, :artifacts]) do
             = link_to project_builds_path(@project), title: 'Jobs', class: 'shortcuts-builds' do
               %span
                 Jobs
diff --git a/app/views/projects/tree/_tree_header.html.haml b/app/views/projects/tree/_tree_header.html.haml
index e7b3fe3ffda..396d1ecd77b 100644
--- a/app/views/projects/tree/_tree_header.html.haml
+++ b/app/views/projects/tree/_tree_header.html.haml
@@ -9,12 +9,9 @@
   %li
     = link_to namespace_project_tree_path(@project.namespace, @project, @ref) do
       = @project.path
-  - tree_breadcrumbs(tree, 6) do |title, path|
+  - path_breadcrumbs do |title, path|
     %li
-      - if path
-        = link_to truncate(title, length: 40), namespace_project_tree_path(@project.namespace, @project, path)
-      - else
-        = link_to title, '#'
+      = link_to truncate(title, length: 40), namespace_project_tree_path(@project.namespace, @project, tree_join(@ref, path))
 
   - if current_user
     %li
diff --git a/changelogs/unreleased/dm-artifact-browser-header.yml b/changelogs/unreleased/dm-artifact-browser-header.yml
new file mode 100644
index 00000000000..b88ab2ac7e5
--- /dev/null
+++ b/changelogs/unreleased/dm-artifact-browser-header.yml
@@ -0,0 +1,4 @@
+---
+title: Add breadcrumb, build header and pipelines submenu to artifacts browser
+merge_request:
+author:
diff --git a/features/project/builds/artifacts.feature b/features/project/builds/artifacts.feature
index 52dc15f2eb6..09094d638c9 100644
--- a/features/project/builds/artifacts.feature
+++ b/features/project/builds/artifacts.feature
@@ -17,6 +17,7 @@ Feature: Project Builds Artifacts
     When I visit recent build details page
     And I click artifacts browse button
     Then I should see content of artifacts archive
+    And I should see the build header
 
   Scenario: I browse subdirectory of build artifacts
     Given recent build has artifacts available
@@ -25,6 +26,7 @@ Feature: Project Builds Artifacts
     And I click artifacts browse button
     And I click link to subdirectory within build artifacts
     Then I should see content of subdirectory within artifacts archive
+    And I should see the directory name in the breadcrumb
 
   Scenario: I browse directory with UTF-8 characters in name
     Given recent build has artifacts available
diff --git a/features/steps/project/builds/artifacts.rb b/features/steps/project/builds/artifacts.rb
index be0f6eee55a..3ec5c8e2f4f 100644
--- a/features/steps/project/builds/artifacts.rb
+++ b/features/steps/project/builds/artifacts.rb
@@ -22,6 +22,12 @@ class Spinach::Features::ProjectBuildsArtifacts < Spinach::FeatureSteps
     end
   end
 
+  step 'I should see the build header' do
+    page.within('.build-header') do
+      expect(page).to have_content "Job ##{@build.id} in pipeline ##{@pipeline.id} for commit #{@pipeline.short_sha}"
+    end
+  end
+
   step 'I click link to subdirectory within build artifacts' do
     page.within('.tree-table') { click_link 'other_artifacts_0.1.2' }
   end
@@ -34,6 +40,12 @@ class Spinach::Features::ProjectBuildsArtifacts < Spinach::FeatureSteps
     end
   end
 
+  step 'I should see the directory name in the breadcrumb' do
+    page.within('.repo-breadcrumb') do
+      expect(page).to have_content 'other_artifacts_0.1.2'
+    end
+  end
+
   step 'recent build artifacts contain directory with UTF-8 characters' do
     # metadata fixture contains relevant directory
   end