diff --git a/app/controllers/admin/runners_controller.rb b/app/controllers/admin/runners_controller.rb index 06880ace89993805a9e6a67efcdce68d671055e6..6d552808f281c74fe323113ea6ac909afd9fd605 100644 --- a/app/controllers/admin/runners_controller.rb +++ b/app/controllers/admin/runners_controller.rb @@ -22,7 +22,7 @@ def show end def edit - assign_builds_and_projects + assign_projects end def update @@ -31,7 +31,7 @@ def update format.html { redirect_to edit_admin_runner_path(@runner) } end else - assign_builds_and_projects + assign_projects render 'show' end end @@ -87,12 +87,7 @@ def permitted_attrs end # rubocop: disable CodeReuse/ActiveRecord - def assign_builds_and_projects - @builds = runner - .builds - .order_id_desc - .preload_project_and_pipeline_project.first(30) - + def assign_projects @projects = if params[:search].present? ::Project.search(params[:search]) diff --git a/app/views/admin/runners/edit.html.haml b/app/views/admin/runners/edit.html.haml index 5570c46c17f8a49bdbaa77b233a049cc4ca2ca4a..ab6ad7090686dbe74641cd792c67aef0476a0638 100644 --- a/app/views/admin/runners/edit.html.haml +++ b/app/views/admin/runners/edit.html.haml @@ -1,5 +1,3 @@ -- add_page_specific_style 'page_bundles/ci_status' - - runner_name = "##{@runner.id} (#{@runner.short_sha})" - if Feature.enabled?(:runner_read_only_admin_view, default_enabled: :yaml) - breadcrumb_title _('Edit') @@ -12,84 +10,47 @@ #js-admin-runner-edit{ data: {runner_id: @runner.id} } -.row - .col-md-6 - %h4= _('Restrict projects for this runner') - - if @runner.runner_projects.any? - %table.table{ data: { testid: 'assigned-projects' } } - %thead - %tr - %th= _('Assigned projects') - - @runner.runner_projects.each do |runner_project| - - project = runner_project.project - - if project - %tr - %td - = render Pajamas::AlertComponent.new(variant: :danger, - dismissible: false, - title: project.full_name) do - .gl-alert-actions - = link_to _('Disable'), admin_namespace_project_runner_project_path(project.namespace, project, runner_project), method: :delete, class: 'btn gl-alert-action btn-confirm btn-md gl-button' - - %table.table{ data: { testid: 'unassigned-projects' } } - %thead - %tr - %th= _('Project') - %th +.gl-overflow-auto + %h4= _('Restrict projects for this runner') +- if @runner.runner_projects.any? + %table.table{ data: { testid: 'assigned-projects' } } + %thead %tr - %td - = form_tag edit_admin_runner_path(@runner), id: 'runner-projects-search', class: 'form-inline', method: :get do - .input-group - = search_field_tag :search, params[:search], class: 'form-control gl-form-input', spellcheck: false - .input-group-append - = submit_tag _('Search'), class: 'gl-button btn btn-default' - - %td - - @projects.each do |project| + %th= _('Assigned projects') + - @runner.runner_projects.each do |runner_project| + - project = runner_project.project + - if project %tr %td - = project.full_name - %td - .float-right - = form_for project.runner_projects.new, url: admin_namespace_project_runner_projects_path(project.namespace, project), method: :post do |f| - = f.hidden_field :runner_id, value: @runner.id - = f.submit _('Enable'), aria: { label: s_('Runners|Change to project runner') }, class: 'gl-button btn btn-sm', data: { confirm: (s_('Runners|You are about to change this instance runner to a project runner. This operation is not reversible. Are you sure you want to continue?') if @runner.instance_type?), confirm_btn_variant: 'danger' } - = paginate_without_count @projects - - .col-md-6 - %h4= _('Recent jobs served by this runner') - %table.table.ci-table.runner-builds - %thead - %tr - %th= _('Job') - %th= _('Status') - %th= _('Project') - %th= _('Commit') - %th= _('Finished at') - - - @builds.each do |build| - - project = build.project - %tr.build - %td.id - - if project - = link_to project_job_path(project, build) do - %strong ##{build.id} - - else - %strong ##{build.id} - - %td.status - = render 'ci/status/badge', status: build.detailed_status(current_user) - - %td.status - - if project - = project.full_name - - %td.build-link - - if project - = link_to pipeline_path(build.pipeline) do - %strong= build.pipeline.short_sha - - %td.timestamp - - if build.finished_at - %span= time_ago_with_tooltip build.finished_at + = render Pajamas::AlertComponent.new(variant: :danger, + dismissible: false, + title: project.full_name) do + .gl-alert-actions + = link_to _('Disable'), admin_namespace_project_runner_project_path(project.namespace, project, runner_project), method: :delete, class: 'btn gl-alert-action btn-confirm btn-md gl-button' + +%table.table{ data: { testid: 'unassigned-projects' } } + %thead + %tr + %th= _('Project') + %th + + %tr + %td + = form_tag edit_admin_runner_path(@runner), id: 'runner-projects-search', class: 'form-inline', method: :get do + .input-group + = search_field_tag :search, params[:search], class: 'form-control gl-form-input', spellcheck: false + .input-group-append + = submit_tag _('Search'), class: 'gl-button btn btn-default' + + %td + - @projects.each do |project| + %tr + %td + = project.full_name + %td + .float-right + = form_for project.runner_projects.new, url: admin_namespace_project_runner_projects_path(project.namespace, project), method: :post do |f| + = f.hidden_field :runner_id, value: @runner.id + = f.submit _('Enable'), aria: { label: s_('Runners|Change to project runner') }, class: 'gl-button btn btn-sm', data: { confirm: (s_('Runners|You are about to change this instance runner to a project runner. This operation is not reversible. Are you sure you want to continue?') if @runner.instance_type?), confirm_btn_variant: 'danger' } += paginate_without_count @projects diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 44aac73a3b7a770653667859eb2fb9f63e71d82b..8d693e81d8f74e2d975aa3fbd206d5ea1f390ba9 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -16065,9 +16065,6 @@ msgstr "" msgid "Finished" msgstr "" -msgid "Finished at" -msgstr "" - msgid "First Name" msgstr "" @@ -30940,9 +30937,6 @@ msgstr "" msgid "Recent events" msgstr "" -msgid "Recent jobs served by this runner" -msgstr "" - msgid "Recent searches" msgstr "" diff --git a/spec/controllers/admin/runners_controller_spec.rb b/spec/controllers/admin/runners_controller_spec.rb index 8f70cb32d3e4d1c823fe4c9b757a7390132d06cd..8bdfbdcb46f1be81d6e77530d44f4a56e833e793 100644 --- a/spec/controllers/admin/runners_controller_spec.rb +++ b/spec/controllers/admin/runners_controller_spec.rb @@ -26,12 +26,6 @@ describe '#show' do render_views - let_it_be(:project) { create(:project) } - - before_all do - create(:ci_build, runner: runner, project: project) - end - it 'shows a runner show page' do get :show, params: { id: runner.id } @@ -55,11 +49,6 @@ let_it_be(:project) { create(:project) } let_it_be(:project_two) { create(:project) } - before_all do - create(:ci_build, runner: runner, project: project) - create(:ci_build, runner: runner, project: project_two) - end - it 'shows a runner edit page' do get :edit, params: { id: runner.id } @@ -77,9 +66,6 @@ control_count = ActiveRecord::QueryRecorder.new { get :edit, params: { id: runner.id } }.count - new_project = create(:project) - create(:ci_build, runner: runner, project: new_project) - # There is one additional query looking up subject.group in ProjectPolicy for the # needs_new_sso_session permission expect { get :edit, params: { id: runner.id } }.not_to exceed_query_limit(control_count + 1) @@ -89,17 +75,42 @@ end describe '#update' do - it 'updates the runner and ticks the queue' do - new_desc = runner.description.swapcase + let(:new_desc) { runner.description.swapcase } + let(:runner_params) { { id: runner.id, runner: { description: new_desc } } } - expect do - post :update, params: { id: runner.id, runner: { description: new_desc } } - end.to change { runner.ensure_runner_queue_value } + subject(:request) { post :update, params: runner_params } - runner.reload + context 'with update succeeding' do + before do + expect_next_instance_of(Ci::Runners::UpdateRunnerService, runner) do |service| + expect(service).to receive(:update).with(anything).and_call_original + end + end - expect(response).to have_gitlab_http_status(:found) - expect(runner.description).to eq(new_desc) + it 'updates the runner and ticks the queue' do + expect { request }.to change { runner.ensure_runner_queue_value } + + runner.reload + + expect(response).to have_gitlab_http_status(:found) + expect(runner.description).to eq(new_desc) + end + end + + context 'with update failing' do + before do + expect_next_instance_of(Ci::Runners::UpdateRunnerService, runner) do |service| + expect(service).to receive(:update).with(anything).and_return(false) + end + end + + it 'does not update runner or tick the queue' do + expect { request }.not_to change { runner.ensure_runner_queue_value } + expect { request }.not_to change { runner.reload.description } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) + end end end