From 294d8588e4ae788333901da82329eceab484905f Mon Sep 17 00:00:00 2001 From: homersimpsons Date: Sat, 6 Jan 2024 21:09:07 +0000 Subject: [PATCH] Add "Delete orphan branches" action --- .rubocop_todo/gitlab/namespaced_class.yml | 1 + .../worker_data_consistency.yml | 1 + .../style/inline_disable_annotation.yml | 1 + ...ged_branches.vue => branches_dropdown.vue} | 100 +++++++++++++----- .../branches/init_branches_dropdown.js | 24 +++++ .../branches/init_delete_merged_branches.js | 23 ---- .../pages/projects/branches/index/index.js | 4 +- .../projects/branches_controller.rb | 9 +- .../branches/delete_orphan_service.rb | 34 ++++++ app/views/projects/branches/index.html.haml | 5 +- app/workers/all_queues.yml | 9 ++ app/workers/delete_orphan_branches_worker.rb | 26 +++++ config/routes/repository.rb | 1 + config/sidekiq_queues.yml | 2 + doc/api/branches.md | 23 ++++ doc/api/openapi/openapi.yaml | 20 ++++ lib/api/branches.rb | 11 ++ 17 files changed, 241 insertions(+), 53 deletions(-) rename app/assets/javascripts/branches/components/{delete_merged_branches.vue => branches_dropdown.vue} (60%) create mode 100644 app/assets/javascripts/branches/init_branches_dropdown.js delete mode 100644 app/assets/javascripts/branches/init_delete_merged_branches.js create mode 100644 app/services/branches/delete_orphan_service.rb create mode 100644 app/workers/delete_orphan_branches_worker.rb diff --git a/.rubocop_todo/gitlab/namespaced_class.yml b/.rubocop_todo/gitlab/namespaced_class.yml index 4d3c8eb4a90a4e..9a7d23e4442c52 100644 --- a/.rubocop_todo/gitlab/namespaced_class.yml +++ b/.rubocop_todo/gitlab/namespaced_class.yml @@ -737,6 +737,7 @@ Gitlab/NamespacedClass: - 'app/workers/create_pipeline_worker.rb' - 'app/workers/delete_diff_files_worker.rb' - 'app/workers/delete_merged_branches_worker.rb' + - 'app/workers/delete_orphan_branches_worker.rb' - 'app/workers/delete_stored_files_worker.rb' - 'app/workers/delete_user_worker.rb' - 'app/workers/destroy_pages_deployments_worker.rb' diff --git a/.rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml b/.rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml index e6612b4830bccd..22894caa194621 100644 --- a/.rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml +++ b/.rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml @@ -83,6 +83,7 @@ SidekiqLoadBalancing/WorkerDataConsistency: - 'app/workers/database/partition_management_worker.rb' - 'app/workers/delete_diff_files_worker.rb' - 'app/workers/delete_merged_branches_worker.rb' + - 'app/workers/delete_orphan_branches_worker.rb' - 'app/workers/delete_stored_files_worker.rb' - 'app/workers/delete_user_worker.rb' - 'app/workers/dependency_proxy/cleanup_blob_worker.rb' diff --git a/.rubocop_todo/style/inline_disable_annotation.yml b/.rubocop_todo/style/inline_disable_annotation.yml index 98690775f3c9d0..39292e2914588f 100644 --- a/.rubocop_todo/style/inline_disable_annotation.yml +++ b/.rubocop_todo/style/inline_disable_annotation.yml @@ -878,6 +878,7 @@ Style/InlineDisableAnnotation: - 'app/workers/database/partition_management_worker.rb' - 'app/workers/delete_diff_files_worker.rb' - 'app/workers/delete_merged_branches_worker.rb' + - 'app/workers/delete_orphan_branches_worker.rb' - 'app/workers/delete_stored_files_worker.rb' - 'app/workers/delete_user_worker.rb' - 'app/workers/dependency_proxy/cleanup_dependency_proxy_worker.rb' diff --git a/app/assets/javascripts/branches/components/delete_merged_branches.vue b/app/assets/javascripts/branches/components/branches_dropdown.vue similarity index 60% rename from app/assets/javascripts/branches/components/delete_merged_branches.vue rename to app/assets/javascripts/branches/components/branches_dropdown.vue index 24ae9b83b9c5b4..1678cf02d0412a 100644 --- a/app/assets/javascripts/branches/components/delete_merged_branches.vue +++ b/app/assets/javascripts/branches/components/branches_dropdown.vue @@ -11,27 +11,38 @@ import csrf from '~/lib/utils/csrf'; import { sprintf, s__, __ } from '~/locale'; export const i18n = { - deleteButtonText: s__('Branches|Delete merged branches'), - modalTitle: s__('Branches|Delete all merged branches?'), - modalMessage: s__( + deleteMergedButtonText: s__('Branches|Delete merged branches'), + modalMergedTitle: s__('Branches|Delete all merged branches?'), + modalMergedMessage: s__( 'Branches|You are about to %{strongStart}delete all branches%{strongEnd} that were merged into %{codeStart}%{defaultBranch}%{codeEnd}.', ), + deleteOrphanButtonText: s__('Branches|Delete orphan branches'), + modalOrphanTitle: s__('Branches|Delete all orphan branches?'), + modalOrphanMessage: s__( + 'Branches|You are about to %{strongStart}delete all branches%{strongEnd} that are not linked to any %{strongStart}open merge request%{strongEnd}.', + ), notVisibleBranchesWarning: s__( - 'Branches|This may include merged branches that are not visible on the current screen.', + 'Branches|This may include branches that are not visible on the current screen.', ), - protectedBranchWarning: s__( + protectedAndMRBranchWarning: s__( "Branches|A branch won't be deleted if it is protected or associated with an open merge request.", ), + protectedBranchWarning: s__( + "Branches|A branch won't be deleted if it is protected.", + ), permanentEffectWarning: s__( 'Branches|This bulk action is %{strongStart}permanent and cannot be undone or recovered%{strongEnd}.', ), confirmationMessage: s__( - 'Branches|Plese type the following to confirm: %{codeStart}delete%{codeEnd}.', + 'Branches|Please type the following to confirm: %{codeStart}delete%{codeEnd}.', ), cancelButtonText: __('Cancel'), actionsToggleText: __('More actions'), }; +const MODE_MERGED = 'merged'; +const MODE_ORPHAN = 'orphan'; + export default { csrf, components: { @@ -45,7 +56,11 @@ export default { GlTooltip: GlTooltipDirective, }, props: { - formPath: { + deleteMergedFormPath: { + type: String, + required: true, + }, + deleteOrphanFormPath: { type: String, required: true, }, @@ -58,13 +73,46 @@ export default { return { areAllBranchesVisible: false, enteredText: '', + mode: MODE_ORPHAN, }; }, computed: { + formPath() { + switch (this.mode) { + case MODE_MERGED: return this.deleteMergedFormPath; + case MODE_ORPHAN: return this.deleteOrphanFormPath; + default: throw Error(`Invaid mode: ${this.mode}`); + } + }, + modalTitle() { + switch (this.mode) { + case MODE_MERGED: return this.$options.i18n.modalMergedTitle; + case MODE_ORPHAN: return this.$options.i18n.modalOrphanTitle; + default: throw Error(`Invaid mode: ${this.mode}`); + } + }, modalMessage() { - return sprintf(this.$options.i18n.modalMessage, { - defaultBranch: this.defaultBranch, - }); + switch (this.mode) { + case MODE_MERGED: return sprintf(this.$options.i18n.modalMergedMessage, { + defaultBranch: this.defaultBranch, + }); + case MODE_ORPHAN: return this.$options.i18n.modalOrphanMessage; + default: throw Error(`Invaid mode: ${this.mode}`); + } + }, + deleteButtonText() { + switch (this.mode) { + case MODE_MERGED: return this.$options.i18n.deleteMergedButtonText; + case MODE_ORPHAN: return this.$options.i18n.deleteOrphanButtonText; + default: throw Error(`Invaid mode: ${this.mode}`); + } + }, + protectedBranchWarning() { + switch (this.mode) { + case MODE_MERGED: return this.$options.i18n.protectedAndMRBranchWarning; + case MODE_ORPHAN: return this.$options.i18n.protectedBranchWarning; + default: throw Error(`Invaid mode: ${this.mode}`); + } }, isDeletingConfirmed() { return this.enteredText.trim().toLowerCase() === 'delete'; @@ -75,9 +123,19 @@ export default { dropdownItems() { return [ { - text: this.$options.i18n.deleteButtonText, + text: this.$options.i18n.deleteMergedButtonText, action: () => { - this.openModal(); + this.openModal(MODE_MERGED); + }, + extraAttrs: { + 'data-testid': 'delete-merged-branches-button', + class: 'gl-text-red-500!', + }, + }, + { + text: this.$options.i18n.deleteOrphanButtonText, + action: () => { + this.openModal(MODE_ORPHAN); }, extraAttrs: { 'data-testid': 'delete-merged-branches-button', @@ -88,7 +146,8 @@ export default { }, }, methods: { - openModal() { + openModal(mode) { + this.mode = mode; this.$refs.modal.show(); }, submitForm() { @@ -120,20 +179,11 @@ export default { class="gl-display-none gl-md-display-block!" :items="dropdownItems" /> - - {{ $options.i18n.deleteButtonText }} -

@@ -150,7 +200,7 @@ export default { {{ $options.i18n.notVisibleBranchesWarning }}

- {{ $options.i18n.protectedBranchWarning }} + {{ protectedBranchWarning }}

@@ -193,7 +243,7 @@ export default { variant="danger" data-testid="delete-merged-branches-confirmation-button" @click="submitForm" - >{{ $options.i18n.deleteButtonText }}{{ $options.i18n.deleteMergedButtonText }} diff --git a/app/assets/javascripts/branches/init_branches_dropdown.js b/app/assets/javascripts/branches/init_branches_dropdown.js new file mode 100644 index 00000000000000..b5ee3691399110 --- /dev/null +++ b/app/assets/javascripts/branches/init_branches_dropdown.js @@ -0,0 +1,24 @@ +import Vue from 'vue'; +import BranchesDropdown from '~/branches/components/branches_dropdown.vue'; + +export default function initDeleteOrphanBranchesModal() { + const el = document.querySelector('.js-branches-dropdown'); + if (!el) { + return false; + } + + const { deleteMergedFormPath, deleteOrphanFormPath, defaultBranch } = el.dataset; + + return new Vue({ + el, + render(createComponent) { + return createComponent(BranchesDropdown, { + props: { + deleteMergedFormPath, + deleteOrphanFormPath, + defaultBranch, + }, + }); + }, + }); +} diff --git a/app/assets/javascripts/branches/init_delete_merged_branches.js b/app/assets/javascripts/branches/init_delete_merged_branches.js deleted file mode 100644 index 998db07d8de08e..00000000000000 --- a/app/assets/javascripts/branches/init_delete_merged_branches.js +++ /dev/null @@ -1,23 +0,0 @@ -import Vue from 'vue'; -import DeleteMergedBranches from '~/branches/components/delete_merged_branches.vue'; - -export default function initDeleteMergedBranchesModal() { - const el = document.querySelector('.js-delete-merged-branches'); - if (!el) { - return false; - } - - const { formPath, defaultBranch } = el.dataset; - - return new Vue({ - el, - render(createComponent) { - return createComponent(DeleteMergedBranches, { - props: { - formPath, - defaultBranch, - }, - }); - }, - }); -} diff --git a/app/assets/javascripts/pages/projects/branches/index/index.js b/app/assets/javascripts/pages/projects/branches/index/index.js index f5cd03ac48db35..3958c58ec1a1f2 100644 --- a/app/assets/javascripts/pages/projects/branches/index/index.js +++ b/app/assets/javascripts/pages/projects/branches/index/index.js @@ -2,7 +2,7 @@ import initDeprecatedRemoveRowBehavior from '~/behaviors/deprecated_remove_row_b import BranchSortDropdown from '~/branches/branch_sort_dropdown'; import initDiverganceGraph from '~/branches/divergence_graph'; import initDeleteBranchModal from '~/branches/init_delete_branch_modal'; -import initDeleteMergedBranches from '~/branches/init_delete_merged_branches'; +import initBranchesDropdown from '~/branches/init_branches_dropdown'; import initBranchMoreActions from '~/branches/init_branch_more_actions'; const { divergingCountsEndpoint, defaultBranch } = document.querySelector( @@ -12,7 +12,7 @@ const { divergingCountsEndpoint, defaultBranch } = document.querySelector( initDiverganceGraph(divergingCountsEndpoint, defaultBranch); BranchSortDropdown(); initDeprecatedRemoveRowBehavior(); -initDeleteMergedBranches(); +initBranchesDropdown(); // TODO Why this is not a VueJS page ? document.querySelectorAll('.js-branch-more-actions').forEach((elem) => initBranchMoreActions(elem)); diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index e60544129ff164..aefb246ec1fed8 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -126,7 +126,14 @@ def destroy_all_merged ::Branches::DeleteMergedService.new(@project, current_user).async_execute redirect_to project_branches_path(@project), - notice: _('Merged branches are being deleted. This can take some time depending on the number of branches. Please refresh the page to see changes.') + notice: _('Merged branches are being deleted. This can take some time depending on the number of branches. Please refresh the page to see changes.') + end + + def destroy_all_orphan + ::Branches::DeleteOrphanService.new(@project, current_user).async_execute + + redirect_to project_branches_path(@project), + notice: _('Orphan branches are being deleted. This can take some time depending on the number of branches. Please refresh the page to see changes.') end private diff --git a/app/services/branches/delete_orphan_service.rb b/app/services/branches/delete_orphan_service.rb new file mode 100644 index 00000000000000..0297be11d9791d --- /dev/null +++ b/app/services/branches/delete_orphan_service.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Branches + class DeleteOrphanService < BaseService + def async_execute + DeleteOrphanBranchesWorker.perform_async(project.id, current_user.id) + end + + def execute + raise Gitlab::Access::AccessDeniedError unless can?(current_user, :push_code, project) + + branches = project.repository.local_branches # TODO: I'm not sure this is the correct action + # Prevent deletion of branches relevant to open merge requests + branches -= merge_request_branch_names + # Prevent deletion of protected branches + branches = branches.reject { |branch| ProtectedBranch.protected?(project, branch) } + + branches.each do |branch| + ::Branches::DeleteService.new(project, current_user).execute(branch) + end + end + + private + + # rubocop: disable CodeReuse/ActiveRecord + def merge_request_branch_names + # reorder(nil) is necessary for SELECT DISTINCT because default scope adds an ORDER BY + source_names = project.origin_merge_requests.opened.reorder(nil).distinct.pluck(:source_branch) + target_names = project.merge_requests.opened.reorder(nil).distinct.pluck(:target_branch) + (source_names + target_names).uniq + end + # rubocop: enable CodeReuse/ActiveRecord + end +end diff --git a/app/views/projects/branches/index.html.haml b/app/views/projects/branches/index.html.haml index c03de6646cf4be..dfce9b58aea009 100644 --- a/app/views/projects/branches/index.html.haml +++ b/app/views/projects/branches/index.html.haml @@ -29,9 +29,10 @@ - if can_push_code? = link_button_to new_project_branch_path(@project), variant: :confirm do = s_('Branches|New branch') - .js-delete-merged-branches.gl-w-7{ data: { + .js-branches-dropdown.gl-w-7{ data: { default_branch: @project.repository.root_ref, - form_path: project_merged_branches_path(@project) } + delete_merged_form_path: project_merged_branches_path(@project), + delete_orphan_form_path: project_orphan_branches_path(@project) } } = render_if_exists 'projects/commits/mirror_status' diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index fc0695c7f6219e..8e8aba50c444e7 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -2865,6 +2865,15 @@ :weight: 1 :idempotent: false :tags: [] +- :name: delete_orphan_branches + :worker_name: DeleteOrphanBranchesWorker + :feature_category: :source_code_management + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: false + :tags: [] - :name: delete_stored_files :worker_name: DeleteStoredFilesWorker :feature_category: :not_owned diff --git a/app/workers/delete_orphan_branches_worker.rb b/app/workers/delete_orphan_branches_worker.rb new file mode 100644 index 00000000000000..78d2b231fd453d --- /dev/null +++ b/app/workers/delete_orphan_branches_worker.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class DeleteOrphanBranchesWorker # rubocop:disable Scalability/IdempotentWorker + include ApplicationWorker + + data_consistency :always + + sidekiq_options retry: 3 + + feature_category :source_code_management + + def perform(project_id, user_id) + begin + project = Project.find(project_id) + rescue ActiveRecord::RecordNotFound + return + end + + user = User.find(user_id) + + begin + ::Branches::DeleteOrphanService.new(project, user).execute + rescue Gitlab::Access::AccessDeniedError + end + end +end diff --git a/config/routes/repository.rb b/config/routes/repository.rb index fc66cb4d6d3230..cb8e8d17d9f95e 100644 --- a/config/routes/repository.rb +++ b/config/routes/repository.rb @@ -51,6 +51,7 @@ end delete :merged_branches, controller: 'branches', action: :destroy_all_merged + delete :orphan_branches, controller: 'branches', action: :destroy_all_orphan resources :tags, only: [:index, :show, :new, :create, :destroy] resources :protected_branches, only: [:index, :show, :create, :update, :destroy, :patch], constraints: { id: Gitlab::PathRegex.git_reference_regex } resources :protected_tags, only: [:index, :show, :create, :update, :destroy] diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index c67e69ab692a5a..266ed214ebc841 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -235,6 +235,8 @@ - 1 - - delete_merged_branches - 1 +- - delete_orphan_branches + - 1 - - delete_stored_files - 1 - - delete_user diff --git a/doc/api/branches.md b/doc/api/branches.md index f5fe7d0526c0d6..534b601bcb7e03 100644 --- a/doc/api/branches.md +++ b/doc/api/branches.md @@ -240,6 +240,29 @@ Example request: curl --request DELETE --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/projects/5/repository/merged_branches" ``` +## Delete orphan branches + +Deletes all orphan branches. + +NOTE: +[Protected branches](../user/project/protected_branches.md) are not deleted as part of this operation. + +```plaintext +DELETE /projects/:id/repository/orphan_branches +``` + +Parameters: + +| Attribute | Type | Required | Description | +|:----------|:---------------|:---------|:-------------------------------------------------------------------------------------------------------------| +| `id` | integer/string | yes | ID or [URL-encoded path of the project](rest/index.md#namespaced-path-encoding) owned by the authenticated user. | + +Example request: + +```shell +curl --request DELETE --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/projects/5/repository/orphan_branches" +``` + ## Related topics - [Branches](../user/project/repository/branches/index.md) diff --git a/doc/api/openapi/openapi.yaml b/doc/api/openapi/openapi.yaml index d89e33a916bb0e..07967083913f30 100644 --- a/doc/api/openapi/openapi.yaml +++ b/doc/api/openapi/openapi.yaml @@ -544,6 +544,26 @@ paths: 404: description: 404 Project Not Found content: {} + /api/v4/projects/{id}/repository/orphan_branches: + delete: + tags: + - branches + description: Delete all orphan branches + operationId: deleteApiV4ProjectsIdRepositoryOrphanBranches + parameters: + - name: id + in: path + description: The ID or URL-encoded path of the project + required: true + schema: + type: string + responses: + 202: + description: 202 Accepted + content: {} + 404: + description: 404 Project Not Found + content: {} /api/v4/projects/{id}/repository/branches/{branch}: get: tags: diff --git a/lib/api/branches.rb b/lib/api/branches.rb index c5ea3a2d3ad7da..e23a9b7b8b30ee 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -223,6 +223,17 @@ class Branches < ::API::Base accepted! end + + desc 'Delete all orphan branches' do + success code: 202, message: '202 Accepted' + failure [{ code: 404, message: '404 Project Not Found' }] + tags %w[branches] + end + delete ':id/repository/orphan_branches' do + ::Branches::DeleteOrphanService.new(user_project, current_user).async_execute + + accepted! + end end end end -- GitLab