diff options
24 files changed, 402 insertions, 31 deletions
diff --git a/app/assets/stylesheets/pages/groups.scss b/app/assets/stylesheets/pages/groups.scss index 6e76db6b68a..c309c8d157a 100644 --- a/app/assets/stylesheets/pages/groups.scss +++ b/app/assets/stylesheets/pages/groups.scss @@ -49,6 +49,12 @@ } } +.save-group-loader { + margin-top: $gl-padding-50; + margin-bottom: $gl-padding-50; + color: $gl-gray-700; +} + .group-nav-container .nav-controls { .group-filter-form { flex: 1 1 auto; diff --git a/app/controllers/groups/imports_controller.rb b/app/controllers/groups/imports_controller.rb new file mode 100644 index 00000000000..b611685f9bc --- /dev/null +++ b/app/controllers/groups/imports_controller.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class Groups::ImportsController < Groups::ApplicationController + include ContinueParams + + def show + if @group.import_state.nil? || @group.import_state.finished? + if continue_params[:to] + redirect_to continue_params[:to], notice: continue_params[:notice] + else + redirect_to group_path(@group), notice: s_('GroupImport|The group was successfully imported.') + end + elsif @group.import_state.failed? + redirect_to new_group_path(@group), alert: s_('GroupImport|Failed to import group.') + else + flash.now[:notice] = continue_params[:notice_now] + end + end +end diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index a75ee3f5439..fba374dbb44 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -74,7 +74,11 @@ class GroupsController < Groups::ApplicationController def show respond_to do |format| format.html do - render_show_html + if @group.import_state&.in_progress? + redirect_to group_import_path(@group) + else + render_show_html + end end format.atom do diff --git a/app/controllers/import/gitlab_groups_controller.rb b/app/controllers/import/gitlab_groups_controller.rb index 9feded2c83f..330af68385e 100644 --- a/app/controllers/import/gitlab_groups_controller.rb +++ b/app/controllers/import/gitlab_groups_controller.rb @@ -8,7 +8,7 @@ class Import::GitlabGroupsController < ApplicationController def create unless file_is_valid?(group_params[:file]) - return redirect_back_or_default(options: { alert: _('Unable to process group import file') }) + return redirect_back_or_default(options: { alert: s_('GroupImport|Unable to process group import file') }) end group_data = group_params.except(:file).merge( @@ -19,15 +19,17 @@ class Import::GitlabGroupsController < ApplicationController group = ::Groups::CreateService.new(current_user, group_data).execute if group.persisted? - Groups::ImportExport::ImportService.new(group: group, user: current_user).async_execute - - redirect_to( - group_path(group), - notice: _("Group '%{group_name}' is being imported.") % { group_name: group.name } - ) + if Groups::ImportExport::ImportService.new(group: group, user: current_user).async_execute + redirect_to( + group_path(group), + notice: s_("GroupImport|Group '%{group_name}' is being imported.") % { group_name: group.name } + ) + else + redirect_to group_path(group), alert: _("Group import could not be scheduled") + end else redirect_back_or_default( - options: { alert: _("Group could not be imported: %{errors}") % { errors: group.errors.full_messages.to_sentence } } + options: { alert: s_("GroupImport|Group could not be imported: %{errors}") % { errors: group.errors.full_messages.to_sentence } } ) end end diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index cc595740696..7cfb4a508da 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -25,8 +25,9 @@ class Projects::BranchesController < Projects::ApplicationController @refs_pipelines = @project.ci_pipelines.latest_successful_for_refs(@branches.map(&:name)) @merged_branch_names = repository.merged_branch_names(@branches.map(&:name)) + @branch_pipeline_statuses = branch_pipeline_statuses - # https://gitlab.com/gitlab-org/gitlab-foss/issues/48097 + # https://gitlab.com/gitlab-org/gitlab/-/issues/22851 Gitlab::GitalyClient.allow_n_plus_1_calls do render end @@ -194,4 +195,15 @@ class Projects::BranchesController < Projects::ApplicationController confidential_issue_project end + + def branch_pipeline_statuses + latest_commits = @branches.map do |branch| + [branch.name, repository.commit(branch.dereferenced_target).sha] + end.to_h + + latest_pipelines = project.ci_pipelines.latest_pipeline_per_commit(latest_commits.values) + latest_commits.transform_values do |commit_sha| + latest_pipelines[commit_sha]&.detailed_status(current_user) + end.compact + end end diff --git a/app/models/group_import_state.rb b/app/models/group_import_state.rb index a4c7092b80f..d22c1ac5550 100644 --- a/app/models/group_import_state.rb +++ b/app/models/group_import_state.rb @@ -32,4 +32,8 @@ class GroupImportState < ApplicationRecord state.update_column(:last_error, last_error) if last_error end end + + def in_progress? + created? || started? + end end diff --git a/app/services/groups/import_export/import_service.rb b/app/services/groups/import_export/import_service.rb index bd611d55847..a5c776f8fc2 100644 --- a/app/services/groups/import_export/import_service.rb +++ b/app/services/groups/import_export/import_service.rb @@ -13,7 +13,16 @@ module Groups end def async_execute - GroupImportWorker.perform_async(current_user.id, group.id) + group_import_state = GroupImportState.safe_find_or_create_by!(group: group) + jid = GroupImportWorker.perform_async(current_user.id, group.id) + + if jid.present? + group_import_state.update!(jid: jid) + else + group_import_state.fail_op('Failed to schedule import job') + + false + end end def execute diff --git a/app/views/groups/imports/show.html.haml b/app/views/groups/imports/show.html.haml new file mode 100644 index 00000000000..ac8ca8797fe --- /dev/null +++ b/app/views/groups/imports/show.html.haml @@ -0,0 +1,10 @@ +- page_title _('Import in progress') +- @content_class = "limit-container-width" unless fluid_layout + +.save-group-loader + .center + %h2 + %i.loading.spinner.spinner-sm + = page_title + %p + = s_('GroupImport|Please wait while we import the group for you. Refresh at will.') diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 26053a25a3e..2e9be28df86 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -29,6 +29,12 @@ .js-branch-divergence-graph .controls.d-none.d-md-block< + - if commit_status + = render 'ci/status/icon', size: 24, status: commit_status, option_css_classes: 'gl-display-inline-flex gl-vertical-align-middle gl-mr-5' + - elsif show_commit_status + .gl-display-inline-flex.gl-vertical-align-middle.gl-mr-5 + %svg.s24 + - if merge_project && create_mr_button?(@repository.root_ref, branch.name) = link_to create_mr_path(@repository.root_ref, branch.name), class: 'btn btn-default' do = _('Merge request') diff --git a/app/views/projects/branches/_panel.html.haml b/app/views/projects/branches/_panel.html.haml index 93061452e12..828371e9656 100644 --- a/app/views/projects/branches/_panel.html.haml +++ b/app/views/projects/branches/_panel.html.haml @@ -12,7 +12,7 @@ = panel_title %ul.content-list.all-branches.qa-all-branches - branches.first(overview_max_branches).each do |branch| - = render "projects/branches/branch", branch: branch, merged: project.repository.merged_to_root_ref?(branch) + = render "projects/branches/branch", branch: branch, merged: project.repository.merged_to_root_ref?(branch), commit_status: @branch_pipeline_statuses[branch.name], show_commit_status: @branch_pipeline_statuses.any? - if branches.size > overview_max_branches .card-footer.text-center = link_to show_more_text, project_branches_filtered_path(project, state: state), id: "state-#{state}", data: { state: state } diff --git a/app/views/projects/branches/index.html.haml b/app/views/projects/branches/index.html.haml index 6bdc6f716fe..ba42f43088f 100644 --- a/app/views/projects/branches/index.html.haml +++ b/app/views/projects/branches/index.html.haml @@ -59,7 +59,7 @@ - elsif @branches.any? %ul.content-list.all-branches - @branches.each do |branch| - = render "projects/branches/branch", branch: branch, merged: @merged_branch_names.include?(branch.name) + = render "projects/branches/branch", branch: branch, merged: @merged_branch_names.include?(branch.name), commit_status: @branch_pipeline_statuses[branch.name], show_commit_status: @branch_pipeline_statuses.any? = paginate @branches, theme: 'gitlab' - else .nothing-here-block diff --git a/app/workers/group_import_worker.rb b/app/workers/group_import_worker.rb index d8f236013bf..36d81468d55 100644 --- a/app/workers/group_import_worker.rb +++ b/app/workers/group_import_worker.rb @@ -9,15 +9,16 @@ class GroupImportWorker # rubocop:disable Scalability/IdempotentWorker def perform(user_id, group_id) current_user = User.find(user_id) group = Group.find(group_id) - group_import = group.build_import_state(jid: self.jid) + group_import_state = group.import_state || group.build_import_state - group_import.start! + group_import_state.jid = self.jid + group_import_state.start! ::Groups::ImportExport::ImportService.new(group: group, user: current_user).execute - group_import.finish! + group_import_state.finish! rescue StandardError => e - group_import&.fail_op(e.message) + group_import_state&.fail_op(e.message) raise e end diff --git a/changelogs/unreleased/jh-group_import_status.yml b/changelogs/unreleased/jh-group_import_status.yml new file mode 100644 index 00000000000..9441ad39216 --- /dev/null +++ b/changelogs/unreleased/jh-group_import_status.yml @@ -0,0 +1,5 @@ +--- +title: Show import in progress screen for group imports +merge_request: 31731 +author: +type: added diff --git a/changelogs/unreleased/show_build_status_in_branch_list.yml b/changelogs/unreleased/show_build_status_in_branch_list.yml new file mode 100644 index 00000000000..259b916af10 --- /dev/null +++ b/changelogs/unreleased/show_build_status_in_branch_list.yml @@ -0,0 +1,5 @@ +--- +title: Show build status on branch list +merge_request: 30948 +author: Lee Tickett +type: added diff --git a/config/routes/group.rb b/config/routes/group.rb index 43314b3f44c..408c57eaa94 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -70,6 +70,7 @@ constraints(::Constraints::GroupUrlConstrainer.new) do end resource :avatar, only: [:destroy] + resource :import, only: [:show] concerns :clusterable diff --git a/doc/administration/server_hooks.md b/doc/administration/server_hooks.md index 8e9aa4d476a..6df0f187a42 100644 --- a/doc/administration/server_hooks.md +++ b/doc/administration/server_hooks.md @@ -68,9 +68,18 @@ Follow the steps below to properly set up a server hook for all repositories: `/home/git/gitlab-shell/hooks`. For Omnibus installs the path is usually `/opt/gitlab/embedded/service/gitlab-shell/hooks`. To look in a different directory for the global custom hooks, - set `custom_hooks_dir` in the GitLab Shell config. For - Omnibus installations, this can be set in `gitlab.rb`; and in source - installations, this can be set in `gitlab-shell/config.yml`. + set `custom_hooks_dir` in the Gitaly config. For Omnibus installations, this is set + in `gitlab.rb`. For source installations, the configuration location depends on the + GitLab version. For: + + - GitLab 13.0 and earlier, this is set in `gitlab-shell/config.yml`. + - GitLab 13.1 and later, this is set in `gitaly/config.toml` under the `[hooks]` + section. + + NOTE: **Note:** + The `custom_hooks_dir` value in `gitlab-shell/config.yml` is still honored in GitLab + 13.1 and later if the value in `gitaly/config.toml` is blank or non-existent. + 1. Create a new directory in this location. Depending on your hook, it will be either a `pre-receive.d`, `post-receive.d`, or `update.d` directory. 1. Inside this new directory, add your hook. Hooks can be diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6b104b05c9a..7c51b215da7 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10928,9 +10928,6 @@ msgstr "" msgid "Group %{group_name} was successfully created." msgstr "" -msgid "Group '%{group_name}' is being imported." -msgstr "" - msgid "Group Audit Events" msgstr "" @@ -10967,9 +10964,6 @@ msgstr "" msgid "Group by:" msgstr "" -msgid "Group could not be imported: %{errors}" -msgstr "" - msgid "Group description" msgstr "" @@ -10997,6 +10991,9 @@ msgstr "" msgid "Group has not been marked for deletion" msgstr "" +msgid "Group import could not be scheduled" +msgstr "" + msgid "Group info:" msgstr "" @@ -11063,6 +11060,24 @@ msgstr "" msgid "GroupActivyMetrics|Recent activity (last 90 days)" msgstr "" +msgid "GroupImport|Failed to import group." +msgstr "" + +msgid "GroupImport|Group '%{group_name}' is being imported." +msgstr "" + +msgid "GroupImport|Group could not be imported: %{errors}" +msgstr "" + +msgid "GroupImport|Please wait while we import the group for you. Refresh at will." +msgstr "" + +msgid "GroupImport|The group was successfully imported." +msgstr "" + +msgid "GroupImport|Unable to process group import file" +msgstr "" + msgid "GroupRoadmap|%{dateWord} – No end date" msgstr "" @@ -23870,9 +23885,6 @@ msgstr "" msgid "Unable to load the merge request widget. Try reloading the page." msgstr "" -msgid "Unable to process group import file" -msgstr "" - msgid "Unable to resolve" msgstr "" diff --git a/spec/controllers/groups/imports_controller_spec.rb b/spec/controllers/groups/imports_controller_spec.rb new file mode 100644 index 00000000000..eb43a62b75b --- /dev/null +++ b/spec/controllers/groups/imports_controller_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Groups::ImportsController do + describe 'GET #show' do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group, :private) } + + context 'when the user has permission to view the group' do + before do + sign_in(user) + group.add_maintainer(user) + end + + context 'when the import is in progress' do + before do + create(:group_import_state, group: group) + end + + it 'renders the show template' do + get :show, params: { group_id: group } + + expect(response).to render_template :show + end + + it 'sets the flash notice' do + get :show, params: { group_id: group, continue: { to: '/', notice_now: 'In progress' } } + + expect(flash.now[:notice]).to eq 'In progress' + end + end + + context 'when the import has failed' do + before do + create(:group_import_state, :failed, group: group) + end + + it 'redirects to the new group path' do + get :show, params: { group_id: group } + + expect(response).to redirect_to new_group_path(group) + end + + it 'sets a flash error' do + get :show, params: { group_id: group } + + expect(flash[:alert]).to eq 'Failed to import group.' + end + end + + context 'when the import has finished' do + before do + create(:group_import_state, :finished, group: group) + end + + it 'redirects to the group page' do + get :show, params: { group_id: group } + + expect(response).to redirect_to group_path(group) + end + end + + context 'when there is no import state' do + it 'redirects to the group page' do + get :show, params: { group_id: group } + + expect(response).to redirect_to group_path(group) + end + end + end + + context 'when the user does not have permission to view the group' do + before do + sign_in(user) + end + + it 'returns a 404' do + get :show, params: { group_id: group } + + expect(response).to have_gitlab_http_status :not_found + end + end + end +end diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index e2e5e07fba0..dce7105c073 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -66,7 +66,19 @@ RSpec.describe GroupsController do subject { get :show, params: { id: group.to_param }, format: format } - it_behaves_like 'details view' + context 'when the group is not importing' do + it_behaves_like 'details view' + end + + context 'when the group is importing' do + before do + create(:group_import_state, group: group) + end + + it 'redirects to the import status page' do + expect(subject).to redirect_to group_import_path(group) + end + end end describe 'GET #details' do diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index 02a8f525b4d..625fc5bddda 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -486,6 +486,82 @@ RSpec.describe Projects::BranchesController do end end + context 'when a branch has multiple pipelines' do + it 'chooses the latest to determine status' do + sha = project.repository.create_file(developer, generate(:branch), 'content', message: 'message', branch_name: 'master') + create(:ci_pipeline, + project: project, + user: developer, + ref: "master", + sha: sha, + status: :running, + created_at: 6.months.ago) + create(:ci_pipeline, + project: project, + user: developer, + ref: "master", + sha: sha, + status: :success, + created_at: 2.months.ago) + + get :index, + format: :html, + params: { + namespace_id: project.namespace, + project_id: project, + state: 'all' + } + + expect(controller.instance_variable_get(:@branch_pipeline_statuses)["master"].group).to eq("success") + end + end + + context 'when multiple branches exist' do + it 'all relevant commit statuses are received' do + master_sha = project.repository.create_file(developer, generate(:branch), 'content', message: 'message', branch_name: 'master') + create(:ci_pipeline, + project: project, + user: developer, + ref: "master", + sha: master_sha, + status: :running, + created_at: 6.months.ago) + test_sha = project.repository.create_file(developer, generate(:branch), 'content', message: 'message', branch_name: 'test') + create(:ci_pipeline, + project: project, + user: developer, + ref: "test", + sha: test_sha, + status: :success, + created_at: 2.months.ago) + + get :index, + format: :html, + params: { + namespace_id: project.namespace, + project_id: project, + state: 'all' + } + + expect(controller.instance_variable_get(:@branch_pipeline_statuses)["master"].group).to eq("running") + expect(controller.instance_variable_get(:@branch_pipeline_statuses)["test"].group).to eq("success") + end + end + + context 'when a branch contains no pipelines' do + it 'no commit statuses are received' do + get :index, + format: :html, + params: { + namespace_id: project.namespace, + project_id: project, + state: 'all' + } + + expect(controller.instance_variable_get(:@branch_pipeline_statuses)).to be_blank + end + end + # We need :request_store because Gitaly only counts the queries whenever # `RequestStore.active?` in GitalyClient.enforce_gitaly_request_limits # And the main goal of this test is making sure TooManyInvocationsError diff --git a/spec/features/projects/branches_spec.rb b/spec/features/projects/branches_spec.rb index 9dc0f7c90c2..28df11e2db5 100644 --- a/spec/features/projects/branches_spec.rb +++ b/spec/features/projects/branches_spec.rb @@ -231,6 +231,44 @@ describe 'Branches' do end end + context 'with one or more pipeline', :js do + before do + sha = create_file(branch_name: "branch") + create(:ci_pipeline, + project: project, + user: user, + ref: "branch", + sha: sha, + status: :success, + created_at: 5.months.ago) + visit project_branches_path(project) + end + + it 'shows pipeline status when available' do + page.within first('.all-branches li') do + expect(page).to have_css 'a.ci-status-icon-success' + end + end + + it 'displays a placeholder when not available' do + page.all('.all-branches li') do |li| + expect(li).to have_css 'svg.s24' + end + end + end + + context 'with no pipelines', :js do + before do + visit project_branches_path(project) + end + + it 'does not show placeholder or pipeline status' do + page.all('.all-branches') do |branches| + expect(branches).not_to have_css 'svg.s24' + end + end + end + describe 'comparing branches' do before do sign_in(user) diff --git a/spec/models/group_import_state_spec.rb b/spec/models/group_import_state_spec.rb index 70884bf420e..9d9cb1e8391 100644 --- a/spec/models/group_import_state_spec.rb +++ b/spec/models/group_import_state_spec.rb @@ -35,4 +35,38 @@ describe GroupImportState do expect(import_state).to be_valid end end + + describe '#in_progress?' do + context "when the import is 'created'" do + it "returns true" do + group_import_state = build(:group_import_state, :created) + + expect(group_import_state.in_progress?).to eq true + end + end + + context "when the import is 'started'" do + it "returns true" do + group_import_state = build(:group_import_state, :started) + + expect(group_import_state.in_progress?).to eq true + end + end + + context "when the import is 'finished'" do + it "returns false" do + group_import_state = build(:group_import_state, :finished) + + expect(group_import_state.in_progress?).to eq false + end + end + + context "when the import is 'failed'" do + it "returns false" do + group_import_state = build(:group_import_state, :failed) + + expect(group_import_state.in_progress?).to eq false + end + end + end end diff --git a/spec/services/groups/import_export/import_service_spec.rb b/spec/services/groups/import_export/import_service_spec.rb index 065be36c1dc..1f7eaccbdbd 100644 --- a/spec/services/groups/import_export/import_service_spec.rb +++ b/spec/services/groups/import_export/import_service_spec.rb @@ -16,6 +16,12 @@ describe Groups::ImportExport::ImportService do import_service.async_execute end + it 'marks the group import as in progress' do + import_service.async_execute + + expect(group.import_state.in_progress?).to eq true + end + it 'returns truthy' do expect(import_service.async_execute).to be_truthy end @@ -31,6 +37,10 @@ describe Groups::ImportExport::ImportService do it 'returns falsey' do expect(import_service.async_execute).to be_falsey end + + it 'does not mark the group import as created' do + expect { import_service.async_execute }.not_to change { group.import_state } + end end end diff --git a/spec/workers/group_import_worker_spec.rb b/spec/workers/group_import_worker_spec.rb index bb7dc116a08..324a5fa6978 100644 --- a/spec/workers/group_import_worker_spec.rb +++ b/spec/workers/group_import_worker_spec.rb @@ -26,7 +26,7 @@ describe GroupImportWorker do subject.perform(user.id, group.id) end - context 'import state' do + context 'when the import state does not exist' do it 'creates group import' do expect(group.import_state).to be_nil @@ -54,6 +54,17 @@ describe GroupImportWorker do subject.perform(user.id, group.id) end end + + context 'when the import state already exists' do + it 'updates the existing state' do + existing_state = create(:group_import_state, group: group) + + expect { subject.perform(user.id, group.id) } + .not_to change { GroupImportState.count } + + expect(existing_state.reload).to be_finished + end + end end context 'when it fails' do |