Merge branch '32551-link-to-breadcrumbs-on-specific-runners-page-is-confusing' into 'master'

Fix breadcrumbs in runner details pages

See merge request gitlab-org/gitlab!60129

.rubocop_manual_todo.yml

@@ -1366,7 +1366,6 @@ RSpec/AnyInstanceOf:
     - 'spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb'
     - 'spec/views/layouts/_head.html.haml_spec.rb'
     - 'spec/views/projects/artifacts/_artifact.html.haml_spec.rb'
-    - 'spec/views/shared/runners/show.html.haml_spec.rb'
     - 'spec/workers/archive_trace_worker_spec.rb'
     - 'spec/workers/build_coverage_worker_spec.rb'
     - 'spec/workers/build_hooks_worker_spec.rb'

.rubocop_todo.yml

@@ -600,7 +600,6 @@ Rails/ShortI18n:
   Exclude:
     - 'app/models/project_services/chat_message/pipeline_message.rb'
     - 'app/uploaders/content_type_whitelist.rb'
-    - 'spec/views/shared/runners/show.html.haml_spec.rb'
 
 # Offense count: 1144
 # Configuration parameters: ForbiddenMethods, AllowedMethods.

app/controllers/groups/runners_controller.rb

@@ -10,7 +10,6 @@ class Groups::RunnersController < Groups::ApplicationController
   feature_category :continuous_integration
 
   def show
-    render 'shared/runners/show'
   end
 
   def edit

app/controllers/projects/runners_controller.rb

@@ -48,7 +48,6 @@ class Projects::RunnersController < Projects::ApplicationController
   end
 
   def show
-    render 'shared/runners/show'
   end
 
   def toggle_shared_runners

app/helpers/groups_helper.rb

@@ -26,6 +26,8 @@ module GroupsHelper
       applications#show
       applications#edit
       packages_and_registries#index
+      groups/runners#show
+      groups/runners#edit
     ]
   end
 

app/helpers/projects_helper.rb

@@ -723,6 +723,8 @@ module ProjectsHelper
       badges#index
       pages#show
       packages_and_registries#index
+      projects/runners#show
+      projects/runners#edit
     ]
   end
 

app/views/admin/runners/show.html.haml

 - add_page_specific_style 'page_bundles/ci_status'
 
-- page_title @runner.short_sha
+- breadcrumb_title @runner.short_sha
+- page_title "##{@runner.id} (#{@runner.short_sha})"
 - add_to_breadcrumbs _('Runners'), admin_runners_path
-- breadcrumb_title page_title
 
 - if Feature.enabled?(:runner_detailed_view_vue_ui, current_user, default_enabled: :yaml)
   #js-runner-detail{ data: {runner_id: @runner.id} }

app/views/groups/runners/edit.html.haml

-- page_title _('Edit'), "#{@runner.description} ##{@runner.id}", _('Runners')
+- breadcrumb_title _('Edit')
+- page_title _('Edit'), "##{@runner.id} (#{@runner.short_sha})"
+- add_to_breadcrumbs _('CI/CD Settings'), group_settings_ci_cd_path(@group)
+- add_to_breadcrumbs "#{@runner.short_sha}", group_runner_path(@group, @runner)
 
 %h2.page-title
   = s_('Runners|Runner #%{runner_id}' % { runner_id: @runner.id })

app/views/groups/runners/show.html.haml

+- add_to_breadcrumbs _('CI/CD Settings'), group_settings_ci_cd_path(@group)
+
+= render 'shared/runners/runner_details', runner: @runner

app/views/layouts/nav/sidebar/_group.html.haml

@@ -170,7 +170,7 @@
                 %span
                   = _('Repository')
 
-            = nav_link(controller: :ci_cd) do
+            = nav_link(controller: [:ci_cd, 'groups/runners']) do
               = link_to group_settings_ci_cd_path(@group), title: _('CI/CD') do
                 %span
                   = _('CI/CD')

app/views/layouts/nav/sidebar/_project_menus.html.haml

@@ -193,7 +193,7 @@
             %span
               = _('Repository')
         - if !@project.archived? && @project.feature_available?(:builds, current_user)
-          = nav_link(controller: :ci_cd) do
+          = nav_link(controller: [:ci_cd, 'projects/runners']) do
             = link_to project_settings_ci_cd_path(@project), title: _('CI/CD') do
               %span
                 = _('CI/CD')

app/views/projects/runners/edit.html.haml

-- page_title _('Edit'), "#{@runner.description} ##{@runner.id}", _('Runners')
+- breadcrumb_title _('Edit')
+- page_title _('Edit'), "##{@runner.id} (#{@runner.short_sha})"
+- add_to_breadcrumbs _('CI/CD Settings'), project_settings_ci_cd_path(@project)
+- add_to_breadcrumbs "#{@runner.short_sha}", project_runner_path(@project, @runner)
 
 %h2.page-title
   = s_('Runners|Runner #%{runner_id}' % { runner_id: @runner.id })

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

+- add_to_breadcrumbs _('CI/CD Settings'), project_settings_ci_cd_path(@project)
+
+= render 'shared/runners/runner_details', runner: @runner

app/views/shared/runners/show.html.haml → app/views/shared/runners/_runner_details.html.haml

-- page_title "#{@runner.description} ##{@runner.id}", _("Runners")
+- breadcrumb_title runner.short_sha
+- page_title "##{runner.id} (#{runner.short_sha})"
 
 %h2.page-title
-  = s_('Runners|Runner #%{runner_id}' % { runner_id: @runner.id })
-  = render 'shared/runners/runner_type_badge', runner: @runner
+  = s_('Runners|Runner #%{runner_id}' % { runner_id: runner.id })
+  = render 'shared/runners/runner_type_badge', runner: runner
 
 .table-holder
   %table.table
@@ -12,51 +13,51 @@
         %th= s_('Runners|Value')
     %tr
       %td= s_('Runners|Active')
-      %td= @runner.active? ? _('Yes') : _('No')
+      %td= runner.active? ? _('Yes') : _('No')
     %tr
       %td= s_('Runners|Protected')
-      %td= @runner.ref_protected? ? _('Yes') : _('No')
+      %td= runner.ref_protected? ? _('Yes') : _('No')
     %tr
       %td= s_('Runners|Can run untagged jobs')
-      %td= @runner.run_untagged? ? _('Yes') : _('No')
-    - unless @runner.group_type?
+      %td= runner.run_untagged? ? _('Yes') : _('No')
+    - unless runner.group_type?
       %tr
         %td= s_('Runners|Locked to this project')
-        %td= @runner.locked? ? _('Yes') : _('No')
+        %td= runner.locked? ? _('Yes') : _('No')
     %tr
       %td= s_('Runners|Tags')
       %td
-        - @runner.tag_list.sort.each do |tag|
+        - runner.tag_list.sort.each do |tag|
           %span.badge.badge-primary
             = tag
     %tr
       %td= s_('Runners|Name')
-      %td= @runner.name
+      %td= runner.name
     %tr
       %td= s_('Runners|Version')
-      %td= @runner.version
+      %td= runner.version
     %tr
       %td= s_('Runners|IP Address')
-      %td= @runner.ip_address
+      %td= runner.ip_address
     %tr
       %td= s_('Runners|Revision')
-      %td= @runner.revision
+      %td= runner.revision
     %tr
       %td= s_('Runners|Platform')
-      %td= @runner.platform
+      %td= runner.platform
     %tr
       %td= s_('Runners|Architecture')
-      %td= @runner.architecture
+      %td= runner.architecture
     %tr
       %td= s_('Runners|Description')
-      %td= @runner.description
+      %td= runner.description
     %tr
       %td= s_('Runners|Maximum job timeout')
-      %td= @runner.maximum_timeout_human_readable
+      %td= runner.maximum_timeout_human_readable
     %tr
       %td= s_('Runners|Last contact')
       %td
-        - if @runner.contacted_at
-          = time_ago_with_tooltip @runner.contacted_at
+        - if runner.contacted_at
+          = time_ago_with_tooltip runner.contacted_at
         - else
           = s_('Never')

changelogs/unreleased/32551-link-to-breadcrumbs-on-specific-runners-page-is-confusing.yml

+---
+title: Fix breadcrumbs and navigation in runner details pages
+merge_request: 60129
+author:
+type: changed

spec/views/shared/runners/show.html.haml_spec.rb → spec/views/shared/runners/_runner_details.html.haml_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'shared/runners/show.html.haml' do
+RSpec.describe 'shared/runners/_runner_details.html.haml' do
   include PageLayoutHelper
 
   let(:runner) do
@@ -14,7 +14,7 @@ RSpec.describe 'shared/runners/show.html.haml' do
   end
 
   before do
-    assign(:runner, runner)
+    allow(view).to receive(:runner) { runner }
   end
 
   subject do
@@ -24,7 +24,7 @@ RSpec.describe 'shared/runners/show.html.haml' do
 
   describe 'Page title' do
     before do
-      expect_any_instance_of(PageLayoutHelper).to receive(:page_title).with("#{runner.description} ##{runner.id}", 'Runners')
+      expect(view).to receive(:page_title).with("##{runner.id} (#{runner.short_sha})")
     end
 
     it 'sets proper page title' do
@@ -147,7 +147,7 @@ RSpec.describe 'shared/runners/show.html.haml' do
 
     context 'when runner have already contacted' do
       let(:runner) { create(:ci_runner, contacted_at: DateTime.now - 6.days) }
-      let(:expected_contacted_at) { I18n.localize(runner.contacted_at, format: "%b %d, %Y") }
+      let(:expected_contacted_at) { I18n.l(runner.contacted_at, format: "%b %d, %Y") }
 
       it { is_expected.to have_content("Last contact #{expected_contacted_at}") }
     end