summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2016-11-04 15:22:36 +0000
committerDouwe Maan <douwe@gitlab.com>2016-11-04 15:22:36 +0000
commitd12e764744e3be11c7ed886fb454e09a4d1f57d6 (patch)
tree66b12fe2442f6ac35597b066b5e32489fc1ff3e9
parent5368b9f249485e254a90fe7daa551d61412bee26 (diff)
parent065ba130585cbce5a2835def94a650d26493abb0 (diff)
downloadgitlab-ce-d12e764744e3be11c7ed886fb454e09a4d1f57d6.tar.gz
Merge branch '20968-add-setting-to-check-unresolved-discussion' into 'master'
Add setting to only allow merge requests to be merged when all discussions are resolved _Originally opened at !6385 by @rodolfoasantos._ - - - ## What does this MR do? Based on #20968 this merge request adds setting only to allow merge requests to be merged when all discussions are resolved. ## Are there points in the code the reviewer needs to double check? Check if there are other points to add the resolved discussion setting ## Why was this MR needed? Add the possibility to configure the project to only accept merge request when all discussions are resolved ## Screenshots ![only_allow_merge_if_all_discussions_are_resolved](/uploads/9388db9421da0214590ffab6fb29f985/only_allow_merge_if_all_discussions_are_resolved.png) ![only_allow_merge_if_all_discussions_are_resolved_msg](/uploads/b1bba0c72ad67d3a1b34718baa08526e/only_allow_merge_if_all_discussions_are_resolved_msg.png) ## Does this MR meet the acceptance criteria? - [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md) - [x] API support added - Tests - [x] Added for this feature/bug - [ ] All builds are passing - [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html) - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if you do - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) ## What are the relevant issue numbers? Closes #20968 See merge request !7125
-rw-r--r--app/controllers/projects_controller.rb1
-rw-r--r--app/models/merge_request.rb7
-rw-r--r--app/views/projects/_merge_request_settings.html.haml4
-rw-r--r--app/views/projects/merge_requests/widget/_open.html.haml4
-rw-r--r--app/views/projects/merge_requests/widget/open/_unresolved_discussions.html.haml6
-rw-r--r--changelogs/unreleased/20968-add-setting-to-check-unresolved-discussion.yml4
-rw-r--r--db/migrate/20160914131004_only_allow_merge_if_all_discussions_are_resolved.rb17
-rw-r--r--db/schema.rb1
-rw-r--r--doc/api/projects.md10
-rw-r--r--doc/user/project/merge_requests/img/only_allow_merge_if_all_discussions_are_resolved.pngbin0 -> 24693 bytes
-rw-r--r--doc/user/project/merge_requests/img/only_allow_merge_if_all_discussions_are_resolved_msg.pngbin0 -> 6940 bytes
-rw-r--r--doc/user/project/merge_requests/merge_request_discussion_resolution.md18
-rw-r--r--doc/user/project/merge_requests/merge_when_build_succeeds.md4
-rw-r--r--lib/api/entities.rb1
-rw-r--r--lib/api/projects.rb9
-rw-r--r--spec/controllers/projects/merge_requests_controller_spec.rb66
-rw-r--r--spec/factories/merge_requests.rb5
-rw-r--r--spec/features/merge_requests/check_if_mergeable_with_unresolved_discussions.rb69
-rw-r--r--spec/models/merge_request_spec.rb59
-rw-r--r--spec/requests/api/projects_spec.rb36
20 files changed, 309 insertions, 12 deletions
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index bce5e29d8d8..6988527a3be 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -335,6 +335,7 @@ class ProjectsController < Projects::ApplicationController
:visibility_level, :import_url, :last_activity_at, :namespace_id, :avatar,
:build_allow_git_fetch, :build_timeout_in_minutes, :build_coverage_regex,
:public_builds, :only_allow_merge_if_build_succeeds, :request_access_enabled,
+ :only_allow_merge_if_all_discussions_are_resolved,
:lfs_enabled, project_feature_attributes
)
end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 6b8ac3fb48b..d76feb9680e 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -425,6 +425,7 @@ class MergeRequest < ActiveRecord::Base
return false if work_in_progress?
return false if broken?
return false unless skip_ci_check || mergeable_ci_state?
+ return false unless mergeable_discussions_state?
true
end
@@ -493,6 +494,12 @@ class MergeRequest < ActiveRecord::Base
discussions_resolvable? && diff_discussions.none?(&:to_be_resolved?)
end
+ def mergeable_discussions_state?
+ return true unless project.only_allow_merge_if_all_discussions_are_resolved?
+
+ discussions_resolved?
+ end
+
def hook_attrs
attrs = {
source: source_project.try(:hook_attrs),
diff --git a/app/views/projects/_merge_request_settings.html.haml b/app/views/projects/_merge_request_settings.html.haml
index 80053dd501b..6e143c4b570 100644
--- a/app/views/projects/_merge_request_settings.html.haml
+++ b/app/views/projects/_merge_request_settings.html.haml
@@ -12,3 +12,7 @@
%span.descr
Builds need to be configured to enable this feature.
= link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_when_build_succeeds', anchor: 'only-allow-merge-requests-to-be-merged-if-the-build-succeeds')
+ .checkbox
+ = f.label :only_allow_merge_if_all_discussions_are_resolved do
+ = f.check_box :only_allow_merge_if_all_discussions_are_resolved
+ %strong Only allow merge requests to be merged if all discussions are resolved
diff --git a/app/views/projects/merge_requests/widget/_open.html.haml b/app/views/projects/merge_requests/widget/_open.html.haml
index 842b6df310d..01314eb37d0 100644
--- a/app/views/projects/merge_requests/widget/_open.html.haml
+++ b/app/views/projects/merge_requests/widget/_open.html.haml
@@ -23,8 +23,10 @@
= render 'projects/merge_requests/widget/open/merge_when_build_succeeds'
- elsif !@merge_request.can_be_merged_by?(current_user)
= render 'projects/merge_requests/widget/open/not_allowed'
- - elsif !@merge_request.mergeable_ci_state? && @pipeline && @pipeline.failed?
+ - elsif !@merge_request.mergeable_ci_state?
= render 'projects/merge_requests/widget/open/build_failed'
+ - elsif !@merge_request.mergeable_discussions_state?
+ = render 'projects/merge_requests/widget/open/unresolved_discussions'
- elsif @merge_request.can_be_merged? || resolved_conflicts
= render 'projects/merge_requests/widget/open/accept'
diff --git a/app/views/projects/merge_requests/widget/open/_unresolved_discussions.html.haml b/app/views/projects/merge_requests/widget/open/_unresolved_discussions.html.haml
new file mode 100644
index 00000000000..35d5677ee37
--- /dev/null
+++ b/app/views/projects/merge_requests/widget/open/_unresolved_discussions.html.haml
@@ -0,0 +1,6 @@
+%h4
+ = icon('exclamation-triangle')
+ This merge request has unresolved discussions
+
+%p
+ Please resolve these discussions to allow this merge request to be merged. \ No newline at end of file
diff --git a/changelogs/unreleased/20968-add-setting-to-check-unresolved-discussion.yml b/changelogs/unreleased/20968-add-setting-to-check-unresolved-discussion.yml
new file mode 100644
index 00000000000..8f03746ff80
--- /dev/null
+++ b/changelogs/unreleased/20968-add-setting-to-check-unresolved-discussion.yml
@@ -0,0 +1,4 @@
+---
+title: Add setting to only allow merge requests to be merged when all discussions are resolved
+merge_request: 7125
+author: Rodolfo Arruda
diff --git a/db/migrate/20160914131004_only_allow_merge_if_all_discussions_are_resolved.rb b/db/migrate/20160914131004_only_allow_merge_if_all_discussions_are_resolved.rb
new file mode 100644
index 00000000000..fad62d716b3
--- /dev/null
+++ b/db/migrate/20160914131004_only_allow_merge_if_all_discussions_are_resolved.rb
@@ -0,0 +1,17 @@
+class OnlyAllowMergeIfAllDiscussionsAreResolved < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+ disable_ddl_transaction!
+
+ def up
+ add_column_with_default(:projects,
+ :only_allow_merge_if_all_discussions_are_resolved,
+ :boolean,
+ default: false)
+ end
+
+ def down
+ remove_column(:projects, :only_allow_merge_if_all_discussions_are_resolved)
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index dc088925d97..5476b0c93e5 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -905,6 +905,7 @@ ActiveRecord::Schema.define(version: 20161103171205) do
t.boolean "has_external_wiki"
t.boolean "lfs_enabled"
t.text "description_html"
+ t.boolean "only_allow_merge_if_all_discussions_are_resolved", default: false, null: false
end
add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree
diff --git a/doc/api/projects.md b/doc/api/projects.md
index 4f4b20a1874..bbb3bfb4995 100644
--- a/doc/api/projects.md
+++ b/doc/api/projects.md
@@ -89,6 +89,7 @@ Parameters:
"public_builds": true,
"shared_with_groups": [],
"only_allow_merge_if_build_succeeds": false,
+ "only_allow_merge_if_all_discussions_are_resolved": false,
"request_access_enabled": false
},
{
@@ -151,6 +152,7 @@ Parameters:
"public_builds": true,
"shared_with_groups": [],
"only_allow_merge_if_build_succeeds": false,
+ "only_allow_merge_if_all_discussions_are_resolved": false,
"request_access_enabled": false
}
]
@@ -429,6 +431,7 @@ Parameters:
}
],
"only_allow_merge_if_build_succeeds": false,
+ "only_allow_merge_if_all_discussions_are_resolved": false,
"request_access_enabled": false
}
```
@@ -602,6 +605,7 @@ Parameters:
| `import_url` | string | no | URL to import repository from |
| `public_builds` | boolean | no | If `true`, builds can be viewed by non-project-members |
| `only_allow_merge_if_build_succeeds` | boolean | no | Set whether merge requests can only be merged with successful builds |
+| `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved |
| `lfs_enabled` | boolean | no | Enable LFS |
| `request_access_enabled` | boolean | no | Allow users to request member access |
@@ -634,6 +638,7 @@ Parameters:
| `import_url` | string | no | URL to import repository from |
| `public_builds` | boolean | no | If `true`, builds can be viewed by non-project-members |
| `only_allow_merge_if_build_succeeds` | boolean | no | Set whether merge requests can only be merged with successful builds |
+| `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved |
| `lfs_enabled` | boolean | no | Enable LFS |
| `request_access_enabled` | boolean | no | Allow users to request member access |
@@ -665,6 +670,7 @@ Parameters:
| `import_url` | string | no | URL to import repository from |
| `public_builds` | boolean | no | If `true`, builds can be viewed by non-project-members |
| `only_allow_merge_if_build_succeeds` | boolean | no | Set whether merge requests can only be merged with successful builds |
+| `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved |
| `lfs_enabled` | boolean | no | Enable LFS |
| `request_access_enabled` | boolean | no | Allow users to request member access |
@@ -752,6 +758,7 @@ Example response:
"public_builds": true,
"shared_with_groups": [],
"only_allow_merge_if_build_succeeds": false,
+ "only_allow_merge_if_all_discussions_are_resolved": false,
"request_access_enabled": false
}
```
@@ -820,6 +827,7 @@ Example response:
"public_builds": true,
"shared_with_groups": [],
"only_allow_merge_if_build_succeeds": false,
+ "only_allow_merge_if_all_discussions_are_resolved": false,
"request_access_enabled": false
}
```
@@ -908,6 +916,7 @@ Example response:
"public_builds": true,
"shared_with_groups": [],
"only_allow_merge_if_build_succeeds": false,
+ "only_allow_merge_if_all_discussions_are_resolved": false,
"request_access_enabled": false
}
```
@@ -996,6 +1005,7 @@ Example response:
"public_builds": true,
"shared_with_groups": [],
"only_allow_merge_if_build_succeeds": false,
+ "only_allow_merge_if_all_discussions_are_resolved": false,
"request_access_enabled": false
}
```
diff --git a/doc/user/project/merge_requests/img/only_allow_merge_if_all_discussions_are_resolved.png b/doc/user/project/merge_requests/img/only_allow_merge_if_all_discussions_are_resolved.png
new file mode 100644
index 00000000000..52c8acf15e0
--- /dev/null
+++ b/doc/user/project/merge_requests/img/only_allow_merge_if_all_discussions_are_resolved.png
Binary files differ
diff --git a/doc/user/project/merge_requests/img/only_allow_merge_if_all_discussions_are_resolved_msg.png b/doc/user/project/merge_requests/img/only_allow_merge_if_all_discussions_are_resolved_msg.png
new file mode 100644
index 00000000000..79ba5c362c7
--- /dev/null
+++ b/doc/user/project/merge_requests/img/only_allow_merge_if_all_discussions_are_resolved_msg.png
Binary files differ
diff --git a/doc/user/project/merge_requests/merge_request_discussion_resolution.md b/doc/user/project/merge_requests/merge_request_discussion_resolution.md
index 2559f5f5250..285b1798ac5 100644
--- a/doc/user/project/merge_requests/merge_request_discussion_resolution.md
+++ b/doc/user/project/merge_requests/merge_request_discussion_resolution.md
@@ -33,7 +33,25 @@ resolved discussions tracker.
!["3/4 discussions resolved"][discussions-resolved]
+## Only allow merge requests to be merged if all discussions are resolved
+
+> [Introduced][ce-7125] in GitLab 8.14.
+
+You can prevent merge requests from being merged until all discussions are resolved.
+
+Navigate to your project's settings page, select the
+**Only allow merge requests to be merged if all discussions are resolved** check
+box and hit **Save** for the changes to take effect.
+
+![Only allow merge if all the discussions are resolved settings](img/only_allow_merge_if_all_discussions_are_resolved.png)
+
+From now on, you will not be able to merge from the UI until all discussions
+are resolved.
+
+![Only allow merge if all the discussions are resolved message](img/only_allow_merge_if_all_discussions_are_resolved_msg.png)
+
[ce-5022]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5022
+[ce-7125]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7125
[resolve-discussion-button]: img/resolve_discussion_button.png
[resolve-comment-button]: img/resolve_comment_button.png
[discussion-view]: img/discussion_view.png
diff --git a/doc/user/project/merge_requests/merge_when_build_succeeds.md b/doc/user/project/merge_requests/merge_when_build_succeeds.md
index c138061fd40..d4e5b5de685 100644
--- a/doc/user/project/merge_requests/merge_when_build_succeeds.md
+++ b/doc/user/project/merge_requests/merge_when_build_succeeds.md
@@ -40,7 +40,7 @@ hit **Save** for the changes to take effect.
![Only allow merge if build succeeds settings](img/merge_when_build_succeeds_only_if_succeeds_settings.png)
-From now on, every time the pipelinefails you will not be able to merge the
+From now on, every time the pipeline fails you will not be able to merge the
merge request from the UI, until you make all relevant builds pass.
-![Only allow merge if build succeeds msg](img/merge_when_build_succeeds_only_if_succeeds_msg.png)
+![Only allow merge if build succeeds message](img/merge_when_build_succeeds_only_if_succeeds_msg.png)
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 1f378ba1635..01e31f6f7d1 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -100,6 +100,7 @@ module API
end
expose :only_allow_merge_if_build_succeeds
expose :request_access_enabled
+ expose :only_allow_merge_if_all_discussions_are_resolved
end
class Member < UserBasic
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index da16e24d7ea..6b856128c2e 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -139,7 +139,8 @@ module API
:shared_runners_enabled,
:snippets_enabled,
:visibility_level,
- :wiki_enabled]
+ :wiki_enabled,
+ :only_allow_merge_if_all_discussions_are_resolved]
attrs = map_public_to_visibility_level(attrs)
@project = ::Projects::CreateService.new(current_user, attrs).execute
if @project.saved?
@@ -193,7 +194,8 @@ module API
:shared_runners_enabled,
:snippets_enabled,
:visibility_level,
- :wiki_enabled]
+ :wiki_enabled,
+ :only_allow_merge_if_all_discussions_are_resolved]
attrs = map_public_to_visibility_level(attrs)
@project = ::Projects::CreateService.new(user, attrs).execute
if @project.saved?
@@ -275,7 +277,8 @@ module API
:shared_runners_enabled,
:snippets_enabled,
:visibility_level,
- :wiki_enabled]
+ :wiki_enabled,
+ :only_allow_merge_if_all_discussions_are_resolved]
attrs = map_public_to_visibility_level(attrs)
authorize_admin_project
authorize! :rename_project, user_project if attrs[:name].present?
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 940d54f8686..49127aecc63 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -297,6 +297,72 @@ describe Projects::MergeRequestsController do
end
end
end
+
+ describe 'only_allow_merge_if_all_discussions_are_resolved? setting' do
+ let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project, author: user) }
+
+ context 'when enabled' do
+ before do
+ project.update_column(:only_allow_merge_if_all_discussions_are_resolved, true)
+ end
+
+ context 'with unresolved discussion' do
+ before do
+ expect(merge_request).not_to be_discussions_resolved
+ end
+
+ it 'returns :failed' do
+ merge_with_sha
+
+ expect(assigns(:status)).to eq(:failed)
+ end
+ end
+
+ context 'with all discussions resolved' do
+ before do
+ merge_request.discussions.each { |d| d.resolve!(user) }
+ expect(merge_request).to be_discussions_resolved
+ end
+
+ it 'returns :success' do
+ merge_with_sha
+
+ expect(assigns(:status)).to eq(:success)
+ end
+ end
+ end
+
+ context 'when disabled' do
+ before do
+ project.update_column(:only_allow_merge_if_all_discussions_are_resolved, false)
+ end
+
+ context 'with unresolved discussion' do
+ before do
+ expect(merge_request).not_to be_discussions_resolved
+ end
+
+ it 'returns :success' do
+ merge_with_sha
+
+ expect(assigns(:status)).to eq(:success)
+ end
+ end
+
+ context 'with all discussions resolved' do
+ before do
+ merge_request.discussions.each { |d| d.resolve!(user) }
+ expect(merge_request).to be_discussions_resolved
+ end
+
+ it 'returns :success' do
+ merge_with_sha
+
+ expect(assigns(:status)).to eq(:success)
+ end
+ end
+ end
+ end
end
end
diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb
index f780e01253c..37eb49c94df 100644
--- a/spec/factories/merge_requests.rb
+++ b/spec/factories/merge_requests.rb
@@ -68,6 +68,11 @@ FactoryGirl.define do
factory :closed_merge_request, traits: [:closed]
factory :reopened_merge_request, traits: [:reopened]
factory :merge_request_with_diffs, traits: [:with_diffs]
+ factory :merge_request_with_diff_notes do
+ after(:create) do |mr|
+ create(:diff_note_on_merge_request, noteable: mr, project: mr.source_project)
+ end
+ end
factory :labeled_merge_request do
transient do
diff --git a/spec/features/merge_requests/check_if_mergeable_with_unresolved_discussions.rb b/spec/features/merge_requests/check_if_mergeable_with_unresolved_discussions.rb
new file mode 100644
index 00000000000..7f11db3c417
--- /dev/null
+++ b/spec/features/merge_requests/check_if_mergeable_with_unresolved_discussions.rb
@@ -0,0 +1,69 @@
+require 'spec_helper'
+
+feature 'Check if mergeable with unresolved discussions', js: true, feature: true do
+ let(:user) { create(:user) }
+ let(:project) { create(:project) }
+ let!(:merge_request) { create(:merge_request_with_diff_notes, source_project: project, author: user) }
+
+ before do
+ login_as user
+ project.team << [user, :master]
+ end
+
+ context 'when project.only_allow_merge_if_all_discussions_are_resolved == true' do
+ before do
+ project.update_column(:only_allow_merge_if_all_discussions_are_resolved, true)
+ end
+
+ context 'with unresolved discussions' do
+ it 'does not allow to merge' do
+ visit_merge_request(merge_request)
+
+ expect(page).not_to have_button 'Accept Merge Request'
+ expect(page).to have_content('This merge request has unresolved discussions')
+ end
+ end
+
+ context 'with all discussions resolved' do
+ before do
+ merge_request.discussions.each { |d| d.resolve!(user) }
+ end
+
+ it 'allows MR to be merged' do
+ visit_merge_request(merge_request)
+
+ expect(page).to have_button 'Accept Merge Request'
+ end
+ end
+ end
+
+ context 'when project.only_allow_merge_if_all_discussions_are_resolved == false' do
+ before do
+ project.update_column(:only_allow_merge_if_all_discussions_are_resolved, false)
+ end
+
+ context 'with unresolved discussions' do
+ it 'does not allow to merge' do
+ visit_merge_request(merge_request)
+
+ expect(page).to have_button 'Accept Merge Request'
+ end
+ end
+
+ context 'with all discussions resolved' do
+ before do
+ merge_request.discussions.each { |d| d.resolve!(user) }
+ end
+
+ it 'allows MR to be merged' do
+ visit_merge_request(merge_request)
+
+ expect(page).to have_button 'Accept Merge Request'
+ end
+ end
+ end
+
+ def visit_merge_request(merge_request)
+ visit namespace_project_merge_request_path(merge_request.project.namespace, merge_request.project, merge_request)
+ end
+end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 1067ff7bb4d..fb032a89d50 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -825,11 +825,8 @@ describe MergeRequest, models: true do
end
context 'when failed' do
- before { allow(subject).to receive(:broken?) { false } }
-
- context 'when project settings restrict to merge only if build succeeds and build failed' do
+ context 'when #mergeable_ci_state? is false' do
before do
- project.only_allow_merge_if_build_succeeds = true
allow(subject).to receive(:mergeable_ci_state?) { false }
end
@@ -837,6 +834,16 @@ describe MergeRequest, models: true do
expect(subject.mergeable_state?).to be_falsey
end
end
+
+ context 'when #mergeable_discussions_state? is false' do
+ before do
+ allow(subject).to receive(:mergeable_discussions_state?) { false }
+ end
+
+ it 'returns false' do
+ expect(subject.mergeable_state?).to be_falsey
+ end
+ end
end
end
@@ -887,7 +894,49 @@ describe MergeRequest, models: true do
end
end
- describe '#environments' do
+ describe '#mergeable_discussions_state?' do
+ let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project) }
+
+ context 'when project.only_allow_merge_if_all_discussions_are_resolved == true' do
+ let(:project) { create(:project, only_allow_merge_if_all_discussions_are_resolved: true) }
+
+ context 'with all discussions resolved' do
+ before do
+ merge_request.discussions.each { |d| d.resolve!(merge_request.author) }
+ end
+
+ it 'returns true' do
+ expect(merge_request.mergeable_discussions_state?).to be_truthy
+ end
+ end
+
+ context 'with unresolved discussions' do
+ before do
+ merge_request.discussions.each(&:unresolve!)
+ end
+
+ it 'returns false' do
+ expect(merge_request.mergeable_discussions_state?).to be_falsey
+ end
+ end
+ end
+
+ context 'when project.only_allow_merge_if_all_discussions_are_resolved == false' do
+ let(:project) { create(:project, only_allow_merge_if_all_discussions_are_resolved: false) }
+
+ context 'with unresolved discussions' do
+ before do
+ merge_request.discussions.each(&:unresolve!)
+ end
+
+ it 'returns true' do
+ expect(merge_request.mergeable_discussions_state?).to be_truthy
+ end
+ end
+ end
+ end
+
+ describe "#environments" do
let(:project) { create(:project) }
let(:merge_request) { create(:merge_request, source_project: project) }
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 973928d007a..3c8f0ac531a 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -256,7 +256,8 @@ describe API::API, api: true do
merge_requests_enabled: false,
wiki_enabled: false,
only_allow_merge_if_build_succeeds: false,
- request_access_enabled: true
+ request_access_enabled: true,
+ only_allow_merge_if_all_discussions_are_resolved: false
})
post api('/projects', user), project
@@ -327,6 +328,22 @@ describe API::API, api: true do
expect(json_response['only_allow_merge_if_build_succeeds']).to be_truthy
end
+ it 'sets a project as allowing merge even if discussions are unresolved' do
+ project = attributes_for(:project, { only_allow_merge_if_all_discussions_are_resolved: false })
+
+ post api('/projects', user), project
+
+ expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to be_falsey
+ end
+
+ it 'sets a project as allowing merge only if all discussions are resolved' do
+ project = attributes_for(:project, { only_allow_merge_if_all_discussions_are_resolved: true })
+
+ post api('/projects', user), project
+
+ expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to be_truthy
+ end
+
context 'when a visibility level is restricted' do
before do
@project = attributes_for(:project, { public: true })
@@ -448,6 +465,22 @@ describe API::API, api: true do
post api("/projects/user/#{user.id}", admin), project
expect(json_response['only_allow_merge_if_build_succeeds']).to be_truthy
end
+
+ it 'sets a project as allowing merge even if discussions are unresolved' do
+ project = attributes_for(:project, { only_allow_merge_if_all_discussions_are_resolved: false })
+
+ post api("/projects/user/#{user.id}", admin), project
+
+ expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to be_falsey
+ end
+
+ it 'sets a project as allowing merge only if all discussions are resolved' do
+ project = attributes_for(:project, { only_allow_merge_if_all_discussions_are_resolved: true })
+
+ post api("/projects/user/#{user.id}", admin), project
+
+ expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to be_truthy
+ end
end
describe "POST /projects/:id/uploads" do
@@ -509,6 +542,7 @@ describe API::API, api: true do
expect(json_response['shared_with_groups'][0]['group_name']).to eq(group.name)
expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access)
expect(json_response['only_allow_merge_if_build_succeeds']).to eq(project.only_allow_merge_if_build_succeeds)
+ expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved)
end
it 'returns a project by path name' do