From 11faf8ae72dcdbaff31f97410a3a9319324438fd Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 23 Oct 2019 21:06:17 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/controllers/projects_controller.rb | 1 + app/graphql/types/project_type.rb | 1 + app/models/project.rb | 1 + app/models/release.rb | 1 + app/presenters/release_presenter.rb | 47 ++++++++++ app/services/merge_requests/build_service.rb | 9 +- ...e_request_merge_options_settings.html.haml | 6 ++ ...msay-configure-default-branch-deletion.yml | 5 ++ ...e_source_branch_after_merge_to_projects.rb | 17 ++++ db/schema.rb | 3 +- doc/api/graphql/reference/index.md | 1 + doc/api/projects.md | 15 ++++ doc/user/project/merge_requests/index.md | 4 +- doc/user/project/settings/index.md | 7 +- lib/api/entities.rb | 41 ++------- lib/api/helpers/projects_helpers.rb | 3 +- .../legacy_upload_mover.rb | 1 + .../legacy_uploads_migrator.rb | 2 +- lib/tasks/gitlab/uploads/legacy.rake | 2 +- locale/gitlab.pot | 6 ++ ...er_manages_merge_requests_settings_spec.rb | 23 +++++ spec/graphql/types/project_type_spec.rb | 1 + .../legacy_upload_mover_spec.rb | 15 +++- .../legacy_uploads_migrator_spec.rb | 12 ++- spec/lib/gitlab/import_export/all_models.yml | 1 + .../import_export/safe_model_attributes.yml | 1 + spec/presenters/release_presenter_spec.rb | 85 +++++++++++++++++++ spec/requests/api/projects_spec.rb | 34 ++++++++ .../merge_requests/build_service_spec.rb | 32 ++++++- 29 files changed, 329 insertions(+), 48 deletions(-) create mode 100644 app/presenters/release_presenter.rb create mode 100644 changelogs/unreleased/jramsay-configure-default-branch-deletion.yml create mode 100644 db/migrate/20191017094449_add_remove_source_branch_after_merge_to_projects.rb create mode 100644 spec/presenters/release_presenter_spec.rb diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index abd19df9a3d..d04801bcfe7 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -371,6 +371,7 @@ class ProjectsController < Projects::ApplicationController :path, :printing_merge_request_link_enabled, :public_builds, + :remove_source_branch_after_merge, :request_access_enabled, :runners_token, :tag_list, diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index 5663f833b7a..3f05610cf1f 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -66,6 +66,7 @@ module Types field :request_access_enabled, GraphQL::BOOLEAN_TYPE, null: true # rubocop:disable Graphql/Descriptions field :only_allow_merge_if_all_discussions_are_resolved, GraphQL::BOOLEAN_TYPE, null: true # rubocop:disable Graphql/Descriptions field :printing_merge_request_link_enabled, GraphQL::BOOLEAN_TYPE, null: true # rubocop:disable Graphql/Descriptions + field :remove_source_branch_after_merge, GraphQL::BOOLEAN_TYPE, null: true, description: 'Remove the source branch by default after merge' field :namespace, Types::NamespaceType, null: true # rubocop:disable Graphql/Descriptions field :group, Types::GroupType, null: true # rubocop:disable Graphql/Descriptions diff --git a/app/models/project.rb b/app/models/project.rb index 9c157b344e8..b5046bbb6ee 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -91,6 +91,7 @@ class Project < ApplicationRecord default_value_for :wiki_enabled, gitlab_config_features.wiki default_value_for :snippets_enabled, gitlab_config_features.snippets default_value_for :only_allow_merge_if_all_discussions_are_resolved, false + default_value_for :remove_source_branch_after_merge, true add_authentication_token_field :runners_token, encrypted: -> { Feature.enabled?(:projects_tokens_optional_encryption, default_enabled: true) ? :optional : :required } diff --git a/app/models/release.rb b/app/models/release.rb index 5a7bfe2d495..a069e1523b8 100644 --- a/app/models/release.rb +++ b/app/models/release.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true class Release < ApplicationRecord + include Presentable include CacheMarkdownField include Gitlab::Utils::StrongMemoize diff --git a/app/presenters/release_presenter.rb b/app/presenters/release_presenter.rb new file mode 100644 index 00000000000..de9055db1e5 --- /dev/null +++ b/app/presenters/release_presenter.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +class ReleasePresenter < Gitlab::View::Presenter::Delegated + include ActionView::Helpers::UrlHelper + + presents :release + + delegate :project, :tag, to: :release + + def commit_path + return unless release.commit && can_download_code? + + project_commit_path(project, release.commit.id) + end + + def tag_path + return unless can_download_code? + + project_tag_path(project, release.tag) + end + + def merge_requests_url + return unless release_mr_issue_urls_available? + + project_merge_requests_url(project, params_for_issues_and_mrs) + end + + def issues_url + return unless release_mr_issue_urls_available? + + project_issues_url(project, params_for_issues_and_mrs) + end + + private + + def can_download_code? + can?(current_user, :download_code, project) + end + + def params_for_issues_and_mrs + { scope: 'all', state: 'opened', release_tag: release.tag } + end + + def release_mr_issue_urls_available? + ::Feature.enabled?(:release_mr_issue_urls, project) + end +end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 214f145d09b..06ee25eac2a 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -10,13 +10,20 @@ module MergeRequests # TODO: this should handle all quick actions that don't have side effects # https://gitlab.com/gitlab-org/gitlab-foss/issues/53658 merge_quick_actions_into_params!(merge_request, only: [:target_branch]) - merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch) if params.has_key?(:force_remove_source_branch) # Assign the projects first so we can use policies for `filter_params` merge_request.author = current_user merge_request.source_project = find_source_project merge_request.target_project = find_target_project + # Source project sets the default source branch removal setting + merge_request.merge_params['force_remove_source_branch'] = + if params.key?(:force_remove_source_branch) + params.delete(:force_remove_source_branch) + else + merge_request.source_project.remove_source_branch_after_merge? + end + filter_params(merge_request) # merge_request.assign_attributes(...) below is a Rails diff --git a/app/views/projects/_merge_request_merge_options_settings.html.haml b/app/views/projects/_merge_request_merge_options_settings.html.haml index 5ab475822de..047b4dafbfc 100644 --- a/app/views/projects/_merge_request_merge_options_settings.html.haml +++ b/app/views/projects/_merge_request_merge_options_settings.html.haml @@ -12,3 +12,9 @@ = form.check_box :printing_merge_request_link_enabled, class: 'form-check-input' = form.label :printing_merge_request_link_enabled, class: 'form-check-label' do = s_('ProjectSettings|Show link to create/view merge request when pushing from the command line') + .form-check.mb-2 + = form.check_box :remove_source_branch_after_merge, class: 'form-check-input' + = form.label :remove_source_branch_after_merge, class: 'form-check-label' do + = s_("ProjectSettings|Enable 'Delete source branch' option by default") + .descr.text-secondary + = s_('ProjectSettings|Existing merge requests and protected branches are not affected') diff --git a/changelogs/unreleased/jramsay-configure-default-branch-deletion.yml b/changelogs/unreleased/jramsay-configure-default-branch-deletion.yml new file mode 100644 index 00000000000..811a78c9dc0 --- /dev/null +++ b/changelogs/unreleased/jramsay-configure-default-branch-deletion.yml @@ -0,0 +1,5 @@ +--- +title: Add project option for deleting source branch +merge_request: 18408 +author: Zsolt Kovari +type: added diff --git a/db/migrate/20191017094449_add_remove_source_branch_after_merge_to_projects.rb b/db/migrate/20191017094449_add_remove_source_branch_after_merge_to_projects.rb new file mode 100644 index 00000000000..021bf7d9870 --- /dev/null +++ b/db/migrate/20191017094449_add_remove_source_branch_after_merge_to_projects.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddRemoveSourceBranchAfterMergeToProjects < ActiveRecord::Migration[5.1] + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + def up + add_column :projects, :remove_source_branch_after_merge, :boolean + end + + def down + remove_column :projects, :remove_source_branch_after_merge + end +end diff --git a/db/schema.rb b/db/schema.rb index 36f7bba904b..31bb3d75ea2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_10_17_045817) do +ActiveRecord::Schema.define(version: 2019_10_17_094449) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" @@ -3058,6 +3058,7 @@ ActiveRecord::Schema.define(version: 2019_10_17_045817) do t.integer "max_pages_size" t.integer "max_artifacts_size" t.string "pull_mirror_branch_prefix", limit: 50 + t.boolean "remove_source_branch_after_merge" t.index "lower((name)::text)", name: "index_projects_on_lower_name" t.index ["archived", "pending_delete", "merge_requests_require_code_owner_approval"], name: "projects_requiring_code_owner_approval", where: "((pending_delete = false) AND (archived = false) AND (merge_requests_require_code_owner_approval = true))" t.index ["created_at"], name: "index_projects_on_created_at" diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 839289cf677..a4fd6bda64e 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -604,6 +604,7 @@ The API can be explored interactively using the [GraphiQL IDE](../index.md#graph | `requestAccessEnabled` | Boolean | | | `onlyAllowMergeIfAllDiscussionsAreResolved` | Boolean | | | `printingMergeRequestLinkEnabled` | Boolean | | +| `removeSourceBranchAfterMerge` | Boolean | Remove the source branch by default after merge | | `namespace` | Namespace | | | `group` | Group | | | `statistics` | ProjectStatistics | | diff --git a/doc/api/projects.md b/doc/api/projects.md index c352b972b17..4497b3e68d3 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -148,6 +148,7 @@ When the user is authenticated and `simple` is not set this returns something li "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, + "remove_source_branch_after_merge": false, "request_access_enabled": false, "merge_method": "merge", "statistics": { @@ -232,6 +233,7 @@ When the user is authenticated and `simple` is not set this returns something li "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, + "remove_source_branch_after_merge": false, "request_access_enabled": false, "merge_method": "merge", "statistics": { @@ -357,6 +359,7 @@ GET /users/:user_id/projects "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, + "remove_source_branch_after_merge": false, "request_access_enabled": false, "merge_method": "merge", "statistics": { @@ -441,6 +444,7 @@ GET /users/:user_id/projects "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, + "remove_source_branch_after_merge": false, "request_access_enabled": false, "merge_method": "merge", "statistics": { @@ -550,6 +554,7 @@ Example response: "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, + "remove_source_branch_after_merge": false, "request_access_enabled": false, "merge_method": "merge", "statistics": { @@ -631,6 +636,7 @@ Example response: "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, + "remove_source_branch_after_merge": false, "request_access_enabled": false, "merge_method": "merge", "statistics": { @@ -757,6 +763,7 @@ GET /projects/:id "repository_storage": "default", "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, + "remove_source_branch_after_merge": false, "printing_merge_requests_link_enabled": true, "request_access_enabled": false, "merge_method": "merge", @@ -917,6 +924,7 @@ POST /projects | `only_allow_merge_if_pipeline_succeeds` | boolean | no | Set whether merge requests can only be merged with successful jobs | | `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved | | `merge_method` | string | no | Set the [merge method](#project-merge-method) used | +| `remove_source_branch_after_merge` | boolean | no | Enable `Delete source branch` option by default for all new merge requests | | `lfs_enabled` | boolean | no | Enable LFS | | `request_access_enabled` | boolean | no | Allow users to request member access | | `tag_list` | array | no | The list of tags for a project; put array of tags, that should be finally assigned to a project | @@ -978,6 +986,7 @@ POST /projects/user/:user_id | `only_allow_merge_if_pipeline_succeeds` | boolean | no | Set whether merge requests can only be merged with successful jobs | | `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved | | `merge_method` | string | no | Set the [merge method](#project-merge-method) used | +| `remove_source_branch_after_merge` | boolean | no | Enable `Delete source branch` option by default for all new merge requests | | `lfs_enabled` | boolean | no | Enable LFS | | `request_access_enabled` | boolean | no | Allow users to request member access | | `tag_list` | array | no | The list of tags for a project; put array of tags, that should be finally assigned to a project | @@ -1039,6 +1048,7 @@ PUT /projects/:id | `only_allow_merge_if_pipeline_succeeds` | boolean | no | Set whether merge requests can only be merged with successful jobs | | `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved | | `merge_method` | string | no | Set the [merge method](#project-merge-method) used | +| `remove_source_branch_after_merge` | boolean | no | Enable `Delete source branch` option by default for all new merge requests | | `lfs_enabled` | boolean | no | Enable LFS | | `request_access_enabled` | boolean | no | Allow users to request member access | | `tag_list` | array | no | The list of tags for a project; put array of tags, that should be finally assigned to a project | @@ -1165,6 +1175,7 @@ Example responses: "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, + "remove_source_branch_after_merge": false, "request_access_enabled": false, "merge_method": "merge", "_links": { @@ -1252,6 +1263,7 @@ Example response: "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, + "remove_source_branch_after_merge": false, "request_access_enabled": false, "merge_method": "merge", "_links": { @@ -1338,6 +1350,7 @@ Example response: "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, + "remove_source_branch_after_merge": false, "request_access_enabled": false, "merge_method": "merge", "_links": { @@ -1511,6 +1524,7 @@ Example response: "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, + "remove_source_branch_after_merge": false, "request_access_enabled": false, "merge_method": "merge", "_links": { @@ -1616,6 +1630,7 @@ Example response: "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, "only_allow_merge_if_all_discussions_are_resolved": false, + "remove_source_branch_after_merge": false, "request_access_enabled": false, "merge_method": "merge", "_links": { diff --git a/doc/user/project/merge_requests/index.md b/doc/user/project/merge_requests/index.md index f673a697c78..00879aae931 100644 --- a/doc/user/project/merge_requests/index.md +++ b/doc/user/project/merge_requests/index.md @@ -94,7 +94,9 @@ You can [search and filter the results](../../search/index.md#issues-and-merge-r When creating a merge request, select the "Delete source branch when merge request accepted" option and the source branch will be deleted when the merge -request is merged. +request is merged. To make this option enabled by default for all new merge +requests, enable it in the +[project's settings](../settings/index.md#merge-request-settings). This option is also visible in an existing merge request next to the merge request button and can be selected/deselected before merging. It's only visible diff --git a/doc/user/project/settings/index.md b/doc/user/project/settings/index.md index 131999dbf60..7c8ae2702a2 100644 --- a/doc/user/project/settings/index.md +++ b/doc/user/project/settings/index.md @@ -49,10 +49,11 @@ Add an [issue description template](../description_templates.md#description-temp Set up your project's merge request settings: - Set up the merge request method (merge commit, [fast-forward merge](../merge_requests/fast_forward_merge.html)). -- Merge request [description templates](../description_templates.md#description-templates). +- Add merge request [description templates](../description_templates.md#description-templates). - Enable [merge request approvals](../merge_requests/merge_request_approvals.md). **(STARTER)** -- Enable [merge only of pipeline succeeds](../merge_requests/merge_when_pipeline_succeeds.md). -- Enable [merge only when all discussions are resolved](../../discussions/index.md#only-allow-merge-requests-to-be-merged-if-all-threads-are-resolved). +- Enable [merge only if pipeline succeeds](../merge_requests/merge_when_pipeline_succeeds.md). +- Enable [merge only when all threads are resolved](../../discussions/index.md#only-allow-merge-requests-to-be-merged-if-all-threads-are-resolved). +- Enable [`delete source branch after merge` option by default](../merge_requests/index.md#deleting-the-source-branch) ![project's merge request settings](img/merge_requests_settings.png) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 91811efacd7..d3a29c6386d 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -307,6 +307,7 @@ module API expose :only_allow_merge_if_pipeline_succeeds expose :request_access_enabled expose :only_allow_merge_if_all_discussions_are_resolved + expose :remove_source_branch_after_merge expose :printing_merge_request_link_enabled expose :merge_method expose :statistics, using: 'API::Entities::ProjectStatistics', if: -> (project, options) { @@ -1290,6 +1291,8 @@ module API end class Release < Grape::Entity + include ::API::Helpers::Presentable + expose :name expose :tag, as: :tag_name, if: ->(_, _) { can_download_code? } expose :description @@ -1302,8 +1305,8 @@ module API expose :commit, using: Entities::Commit, if: ->(_, _) { can_download_code? } expose :upcoming_release?, as: :upcoming_release expose :milestones, using: Entities::Milestone, if: -> (release, _) { release.milestones.present? } - expose :commit_path, if: ->(_, _) { can_download_code? } - expose :tag_path, if: ->(_, _) { can_download_code? } + expose :commit_path, expose_nil: false + expose :tag_path, expose_nil: false expose :assets do expose :assets_count, as: :count do |release, _| assets_to_exclude = can_download_code? ? [] : [:sources] @@ -1315,8 +1318,8 @@ module API end end expose :_links do - expose :merge_requests_url, if: -> (_) { release_mr_issue_urls_available? } - expose :issues_url, if: -> (_) { release_mr_issue_urls_available? } + expose :merge_requests_url, expose_nil: false + expose :issues_url, expose_nil: false end private @@ -1324,36 +1327,6 @@ module API def can_download_code? Ability.allowed?(options[:current_user], :download_code, object.project) end - - def commit_path - return unless object.commit - - Gitlab::Routing.url_helpers.project_commit_path(project, object.commit.id) - end - - def tag_path - Gitlab::Routing.url_helpers.project_tag_path(project, object.tag) - end - - def merge_requests_url - Gitlab::Routing.url_helpers.project_merge_requests_url(project, params_for_issues_and_mrs) - end - - def issues_url - Gitlab::Routing.url_helpers.project_issues_url(project, params_for_issues_and_mrs) - end - - def params_for_issues_and_mrs - { scope: 'all', state: 'opened', release_tag: object.tag } - end - - def release_mr_issue_urls_available? - ::Feature.enabled?(:release_mr_issue_urls, project) - end - - def project - @project ||= object.project - end end class Tag < Grape::Entity diff --git a/lib/api/helpers/projects_helpers.rb b/lib/api/helpers/projects_helpers.rb index 94619204274..47b1f037eb8 100644 --- a/lib/api/helpers/projects_helpers.rb +++ b/lib/api/helpers/projects_helpers.rb @@ -30,6 +30,7 @@ module API optional :shared_runners_enabled, type: Boolean, desc: 'Flag indication if shared runners are enabled for that project' optional :resolve_outdated_diff_discussions, type: Boolean, desc: 'Automatically resolve merge request diffs discussions on lines changed with a push' + optional :remove_source_branch_after_merge, type: Boolean, desc: 'Remove the source branch by default after merge' optional :container_registry_enabled, type: Boolean, desc: 'Flag indication if the container registry is enabled for that project' optional :lfs_enabled, type: Boolean, desc: 'Flag indication if Git LFS is enabled for that project' optional :visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The visibility of the project.' @@ -94,6 +95,7 @@ module API :path, :printing_merge_request_link_enabled, :public_builds, + :remove_source_branch_after_merge, :repository_access_level, :request_access_enabled, :resolve_outdated_diff_discussions, @@ -109,7 +111,6 @@ module API :jobs_enabled, :merge_requests_enabled, :wiki_enabled, - :jobs_enabled, :snippets_enabled ] end diff --git a/lib/gitlab/background_migration/legacy_upload_mover.rb b/lib/gitlab/background_migration/legacy_upload_mover.rb index c9e47f210be..1879a6c5427 100644 --- a/lib/gitlab/background_migration/legacy_upload_mover.rb +++ b/lib/gitlab/background_migration/legacy_upload_mover.rb @@ -18,6 +18,7 @@ module Gitlab def execute return unless upload + return unless upload.model_type == 'Note' if !project # if we don't have models associated with the upload we can not move it diff --git a/lib/gitlab/background_migration/legacy_uploads_migrator.rb b/lib/gitlab/background_migration/legacy_uploads_migrator.rb index a9d38a27e0c..f7cadb9b00d 100644 --- a/lib/gitlab/background_migration/legacy_uploads_migrator.rb +++ b/lib/gitlab/background_migration/legacy_uploads_migrator.rb @@ -14,7 +14,7 @@ module Gitlab include Database::MigrationHelpers def perform(start_id, end_id) - Upload.where(id: start_id..end_id, uploader: 'AttachmentUploader').find_each do |upload| + Upload.where(id: start_id..end_id, uploader: 'AttachmentUploader', model_type: 'Note').find_each do |upload| LegacyUploadMover.new(upload).execute end end diff --git a/lib/tasks/gitlab/uploads/legacy.rake b/lib/tasks/gitlab/uploads/legacy.rake index 2eeb694d341..74db0060b8d 100644 --- a/lib/tasks/gitlab/uploads/legacy.rake +++ b/lib/tasks/gitlab/uploads/legacy.rake @@ -15,7 +15,7 @@ namespace :gitlab do batch_size = 5000 delay_interval = 5.minutes.to_i - Upload.where(uploader: 'AttachmentUploader').each_batch(of: batch_size) do |relation, index| + Upload.where(uploader: 'AttachmentUploader', model_type: 'Note').each_batch(of: batch_size) do |relation, index| start_id, end_id = relation.pluck('MIN(id), MAX(id)').first delay = index * delay_interval diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 083e8f2a7fd..136cd00ce6c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -12857,9 +12857,15 @@ msgstr "" msgid "ProjectSettings|Customize your project badges." msgstr "" +msgid "ProjectSettings|Enable 'Delete source branch' option by default" +msgstr "" + msgid "ProjectSettings|Every merge creates a merge commit" msgstr "" +msgid "ProjectSettings|Existing merge requests and protected branches are not affected" +msgstr "" + msgid "ProjectSettings|Failed to protect the tag" msgstr "" diff --git a/spec/features/projects/settings/user_manages_merge_requests_settings_spec.rb b/spec/features/projects/settings/user_manages_merge_requests_settings_spec.rb index 9f09c5c4501..c0089e3c28c 100644 --- a/spec/features/projects/settings/user_manages_merge_requests_settings_spec.rb +++ b/spec/features/projects/settings/user_manages_merge_requests_settings_spec.rb @@ -107,4 +107,27 @@ describe 'Projects > Settings > User manages merge request settings' do expect(project.printing_merge_request_link_enabled).to be(false) end end + + describe 'Checkbox to remove source branch after merge', :js do + it 'is initially checked' do + checkbox = find_field('project_remove_source_branch_after_merge') + expect(checkbox).to be_checked + end + + it 'when unchecked sets :remove_source_branch_after_merge to false' do + uncheck('project_remove_source_branch_after_merge') + within('.merge-request-settings-form') do + find('.qa-save-merge-request-changes') + click_on('Save changes') + end + + find('.flash-notice') + checkbox = find_field('project_remove_source_branch_after_merge') + + expect(checkbox).not_to be_checked + + project.reload + expect(project.remove_source_branch_after_merge).to be(false) + end + end end diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index cfd0f8ec7a7..f837fb9f77f 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -23,6 +23,7 @@ describe GitlabSchema.types['Project'] do only_allow_merge_if_all_discussions_are_resolved printing_merge_request_link_enabled namespace group statistics repository merge_requests merge_request issues issue pipelines + removeSourceBranchAfterMerge ] is_expected.to have_graphql_fields(*expected_fields) diff --git a/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb b/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb index c1eaf1d3433..f2de73d5aea 100644 --- a/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb +++ b/spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb @@ -91,15 +91,26 @@ describe Gitlab::BackgroundMigration::LegacyUploadMover do end end - context 'when no model found for the upload' do + context 'when no note found for the upload' do before do - legacy_upload.model = nil + legacy_upload.model_id = nil + legacy_upload.model_type = 'Note' expect_error_log end it_behaves_like 'legacy upload deletion' end + context 'when upload does not belong to a note' do + before do + legacy_upload.model = create(:appearance) + end + + it 'does not remove the upload' do + expect { described_class.new(legacy_upload).execute }.not_to change { Upload.count } + end + end + context 'when the upload move fails' do before do expect(FileUploader).to receive(:copy_to).and_raise('failed') diff --git a/spec/lib/gitlab/background_migration/legacy_uploads_migrator_spec.rb b/spec/lib/gitlab/background_migration/legacy_uploads_migrator_spec.rb index cabca3dbef9..85187d039c1 100644 --- a/spec/lib/gitlab/background_migration/legacy_uploads_migrator_spec.rb +++ b/spec/lib/gitlab/background_migration/legacy_uploads_migrator_spec.rb @@ -35,6 +35,8 @@ describe Gitlab::BackgroundMigration::LegacyUploadsMigrator do let!(:legacy_upload_no_file) { create_upload(note2, false) } let!(:legacy_upload_legacy_project) { create_upload(note_legacy) } + let!(:appearance) { create(:appearance, :with_logo) } + let(:start_id) { 1 } let(:end_id) { 10000 } @@ -52,12 +54,18 @@ describe Gitlab::BackgroundMigration::LegacyUploadsMigrator do expect(File.exist?(legacy_upload_legacy_project.absolute_path)).to be_falsey end - it 'removes all AttachmentUploader records' do - expect { subject }.to change { Upload.where(uploader: 'AttachmentUploader').count }.from(3).to(0) + it 'removes all Note AttachmentUploader records' do + expect { subject }.to change { Upload.where(uploader: 'AttachmentUploader').count }.from(4).to(1) end it 'creates new uploads for successfully migrated records' do expect { subject }.to change { Upload.where(uploader: 'FileUploader').count }.from(0).to(2) end + + it 'does not remove appearance uploads' do + subject + + expect(appearance.logo.file).to exist + end end # rubocop: enable RSpec/FactoriesInMigrationSpecs diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 4fd61383c6b..2e3bc4606b9 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -421,6 +421,7 @@ project: - pages_metadatum - alerts_service - grafana_integration +- remove_source_branch_after_merge award_emoji: - awardable - user diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 16f4115fc6e..ede8eb4b2bd 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -512,6 +512,7 @@ Project: - request_access_enabled - has_external_wiki - only_allow_merge_if_all_discussions_are_resolved +- remove_source_branch_after_merge - auto_cancel_pending_pipelines - printing_merge_request_link_enabled - resolve_outdated_diff_discussions diff --git a/spec/presenters/release_presenter_spec.rb b/spec/presenters/release_presenter_spec.rb new file mode 100644 index 00000000000..8bca38f7054 --- /dev/null +++ b/spec/presenters/release_presenter_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ReleasePresenter do + include Gitlab::Routing.url_helpers + + let_it_be(:project) { create(:project, :repository) } + let(:developer) { create(:user) } + let(:guest) { create(:user) } + let(:user) { developer } + let(:release) { create(:release, project: project) } + let(:presenter) { described_class.new(release, current_user: user) } + + before do + project.add_developer(developer) + project.add_guest(guest) + end + + describe '#commit_path' do + subject { presenter.commit_path } + + it 'returns commit path' do + is_expected.to eq(project_commit_path(project, release.commit.id)) + end + + context 'when commit is not found' do + let(:release) { create(:release, project: project, sha: 'not-found') } + + it { is_expected.to be_nil } + end + + context 'when user is guest' do + let(:user) { guest } + + it { is_expected.to be_nil } + end + end + + describe '#tag_path' do + subject { presenter.tag_path } + + it 'returns tag path' do + is_expected.to eq(project_tag_path(project, release.tag)) + end + + context 'when user is guest' do + let(:user) { guest } + + it { is_expected.to be_nil } + end + end + + describe '#merge_requests_url' do + subject { presenter.merge_requests_url } + + it 'returns merge requests url' do + is_expected.to match /#{project_merge_requests_url(project)}/ + end + + context 'when release_mr_issue_urls feature flag is disabled' do + before do + stub_feature_flags(release_mr_issue_urls: false) + end + + it { is_expected.to be_nil } + end + end + + describe '#issues_url' do + subject { presenter.issues_url } + + it 'returns merge requests url' do + is_expected.to match /#{project_issues_url(project)}/ + end + + context 'when release_mr_issue_urls feature flag is disabled' do + before do + stub_feature_flags(release_mr_issue_urls: false) + end + + it { is_expected.to be_nil } + end + end +end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index a4ef050a698..0e343210f5e 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -606,6 +606,7 @@ describe API::Projects do merge_requests_enabled: false, wiki_enabled: false, resolve_outdated_diff_discussions: false, + remove_source_branch_after_merge: true, only_allow_merge_if_pipeline_succeeds: false, request_access_enabled: true, only_allow_merge_if_all_discussions_are_resolved: false, @@ -722,6 +723,22 @@ describe API::Projects do expect(json_response['resolve_outdated_diff_discussions']).to be_truthy end + it 'sets a project as not removing source branches' do + project = attributes_for(:project, remove_source_branch_after_merge: false) + + post api('/projects', user), params: project + + expect(json_response['remove_source_branch_after_merge']).to be_falsey + end + + it 'sets a project as removing source branches' do + project = attributes_for(:project, remove_source_branch_after_merge: true) + + post api('/projects', user), params: project + + expect(json_response['remove_source_branch_after_merge']).to be_truthy + end + it 'sets a project as allowing merge even if build fails' do project = attributes_for(:project, only_allow_merge_if_pipeline_succeeds: false) @@ -980,6 +997,22 @@ describe API::Projects do expect(json_response['resolve_outdated_diff_discussions']).to be_truthy end + it 'sets a project as not removing source branches' do + project = attributes_for(:project, remove_source_branch_after_merge: false) + + post api("/projects/user/#{user.id}", admin), params: project + + expect(json_response['remove_source_branch_after_merge']).to be_falsey + end + + it 'sets a project as removing source branches' do + project = attributes_for(:project, remove_source_branch_after_merge: true) + + post api("/projects/user/#{user.id}", admin), params: project + + expect(json_response['remove_source_branch_after_merge']).to be_truthy + end + it 'sets a project as allowing merge even if build fails' do project = attributes_for(:project, only_allow_merge_if_pipeline_succeeds: false) post api("/projects/user/#{user.id}", admin), params: project @@ -1157,6 +1190,7 @@ describe API::Projects do expect(json_response['wiki_access_level']).to be_present expect(json_response['builds_access_level']).to be_present expect(json_response['resolve_outdated_diff_discussions']).to eq(project.resolve_outdated_diff_discussions) + expect(json_response['remove_source_branch_after_merge']).to be_truthy expect(json_response['container_registry_enabled']).to be_present expect(json_response['created_at']).to be_present expect(json_response['last_activity_at']).to be_present diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index 61cb60e1536..46e86d5b4cb 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -80,7 +80,7 @@ describe MergeRequests::BuildService do end it 'does not assign force_remove_source_branch' do - expect(merge_request.force_remove_source_branch?).to be_falsey + expect(merge_request.force_remove_source_branch?).to be_truthy end context 'with force_remove_source_branch parameter' do @@ -90,6 +90,36 @@ describe MergeRequests::BuildService do it 'assigns force_remove_source_branch' do expect(merge_request.force_remove_source_branch?).to be_truthy end + + context 'with project setting remove_source_branch_after_merge false' do + before do + project.remove_source_branch_after_merge = false + end + + it 'assigns force_remove_source_branch' do + expect(merge_request.force_remove_source_branch?).to be_truthy + end + end + end + + context 'with project setting remove_source_branch_after_merge true' do + before do + project.remove_source_branch_after_merge = true + end + + it 'assigns force_remove_source_branch' do + expect(merge_request.force_remove_source_branch?).to be_truthy + end + + context 'with force_remove_source_branch parameter false' do + before do + params[:force_remove_source_branch] = '0' + end + + it 'does not assign force_remove_source_branch' do + expect(merge_request.force_remove_source_branch?).to be(false) + end + end end context 'missing source branch' do