Commit 48fe224e authored by Ashley Dumaine's avatar Ashley Dumaine
Browse files

change collapse to resolve and comments to discussions

parent ecfd82ac909a
......@@ -323,7 +323,7 @@
:build_allow_git_fetch,
:build_coverage_regex,
:build_timeout_in_minutes,
:collapse_outdated_diff_comments,
:resolve_outdated_diff_discussions,
:container_registry_enabled,
:default_branch,
:description,
......
......@@ -18,14 +18,6 @@
false
end
def collapsed?
resolved? || (project.collapse_outdated_diff_comments && !active?)
end
def expanded?
!collapsed?
end
def merge_request_version_params
return unless for_merge_request?
return {} if active?
......
......@@ -37,7 +37,7 @@
default_value_for :archived, false
default_value_for :visibility_level, gitlab_config_features.visibility_level
default_value_for :collapse_outdated_diff_comments, false
default_value_for :resolve_outdated_diff_discussions, false
default_value_for :container_registry_enabled, gitlab_config_features.container_registry
default_value_for(:repository_storage) { current_application_settings.pick_repository_storage }
default_value_for(:shared_runners_enabled) { current_application_settings.shared_runners_enabled }
......
- if discussion.resolved?
.discussion-headline-light.js-discussion-headline
Resolved
- if discussion.resolved_by
by
= link_to_member(@project, discussion.resolved_by, avatar: false)
= time_ago_with_tooltip(discussion.resolved_at, placement: "bottom")
- if project.resolve_outdated_diff_discussions
.discussion-headline-light.js-discussion-headline
Automatically resolved
- if discussion.resolved_by
by
= link_to_member(@project, discussion.resolved_by, avatar: false)
with a push
= time_ago_with_tooltip(discussion.resolved_at, placement: "bottom")
- else
.discussion-headline-light.js-discussion-headline
Resolved
- if discussion.resolved_by
by
= link_to_member(@project, discussion.resolved_by, avatar: false)
= time_ago_with_tooltip(discussion.resolved_at, placement: "bottom")
- elsif discussion.updated?
.discussion-headline-light.js-discussion-headline
Last updated
......
......@@ -14,6 +14,10 @@
= form.check_box :only_allow_merge_if_all_discussions_are_resolved
%strong Only allow merge requests to be merged if all discussions are resolved
.checkbox
= form.label :resolve_outdated_diff_discussions do
= form.check_box :resolve_outdated_diff_discussions
%strong Automatically resolve merge request diffs discussions on lines changed with a push
.checkbox
= form.label :printing_merge_request_link_enabled do
= form.check_box :printing_merge_request_link_enabled
%strong Show link to create/view merge request when pushing from the command line
......@@ -17,7 +21,3 @@
= form.label :printing_merge_request_link_enabled do
= form.check_box :printing_merge_request_link_enabled
%strong Show link to create/view merge request when pushing from the command line
.checkbox
= form.label :collapse_outdated_diff_comments do
= form.check_box :collapse_outdated_diff_comments
%strong Collapse outdated diffs regardless of discussion resolution
---
title: Add repository toggle for automatically collapsing outdated diff comments regardless
of discussion resolution
title: Add repository toggle for automatically resolving outdated diff discussions
merge_request:
author:
type: added
# rubocop:disable Migration/AddColumnWithDefaultToLargeTable
class CollapseOutdatedDiffComments < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_column_with_default(:projects,
:collapse_outdated_diff_comments,
:boolean,
default: false)
end
def down
remove_column(:projects, :collapse_outdated_diff_comments)
end
end
class CollapseOutdatedDiffComments < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_column_with_default(:projects,
:resolve_outdated_diff_discussions,
:boolean,
default: false)
end
def down
remove_column(:projects, :resolve_outdated_diff_discussions)
end
end
......@@ -1202,7 +1202,7 @@
t.boolean "public_builds", default: true, null: false
t.boolean "last_repository_check_failed"
t.datetime "last_repository_check_at"
t.boolean "collapse_outdated_diff_comments", default: false, null: false
t.boolean "resolve_outdated_diff_discussions", default: false, null: false
t.boolean "container_registry_enabled"
t.boolean "only_allow_merge_if_pipeline_succeeds", default: false, null: false
t.boolean "has_external_issue_tracker"
......
......@@ -70,7 +70,7 @@
"jobs_enabled": true,
"wiki_enabled": true,
"snippets_enabled": false,
"collapse_outdated_diff_comments": false,
"resolve_outdated_diff_discussions": false,
"container_registry_enabled": false,
"created_at": "2013-09-30T13:46:02Z",
"last_activity_at": "2013-09-30T13:46:02Z",
......@@ -138,7 +138,7 @@
"jobs_enabled": true,
"wiki_enabled": true,
"snippets_enabled": false,
"collapse_outdated_diff_comments": false,
"resolve_outdated_diff_discussions": false,
"container_registry_enabled": false,
"created_at": "2013-09-30T13:46:02Z",
"last_activity_at": "2013-09-30T13:46:02Z",
......@@ -248,7 +248,7 @@
"jobs_enabled": true,
"wiki_enabled": true,
"snippets_enabled": false,
"collapse_outdated_diff_comments": false,
"resolve_outdated_diff_discussions": false,
"container_registry_enabled": false,
"created_at": "2013-09-30T13:46:02Z",
"last_activity_at": "2013-09-30T13:46:02Z",
......@@ -316,7 +316,7 @@
"jobs_enabled": true,
"wiki_enabled": true,
"snippets_enabled": false,
"collapse_outdated_diff_comments": false,
"resolve_outdated_diff_discussions": false,
"container_registry_enabled": false,
"created_at": "2013-09-30T13:46:02Z",
"last_activity_at": "2013-09-30T13:46:02Z",
......@@ -415,7 +415,7 @@
"jobs_enabled": true,
"wiki_enabled": true,
"snippets_enabled": false,
"collapse_outdated_diff_comments": false,
"resolve_outdated_diff_discussions": false,
"container_registry_enabled": false,
"created_at": "2013-09-30T13:46:02Z",
"last_activity_at": "2013-09-30T13:46:02Z",
......@@ -542,7 +542,7 @@
| `jobs_enabled` | boolean | no | Enable jobs for this project |
| `wiki_enabled` | boolean | no | Enable wiki for this project |
| `snippets_enabled` | boolean | no | Enable snippets for this project |
| `collapse_outdated_diff_comments` | boolean | no | Collapse outdated diffs regardless of discussion resolution |
| `resolve_outdated_diff_discussions` | boolean | no | Automatically resolve merge request diffs discussions on lines changed with a push |
| `container_registry_enabled` | boolean | no | Enable container registry for this project |
| `shared_runners_enabled` | boolean | no | Enable shared runners for this project |
| `visibility` | string | no | See [project visibility level](#project-visibility-level) |
......@@ -580,7 +580,7 @@
| `jobs_enabled` | boolean | no | Enable jobs for this project |
| `wiki_enabled` | boolean | no | Enable wiki for this project |
| `snippets_enabled` | boolean | no | Enable snippets for this project |
| `collapse_outdated_diff_comments` | boolean | no | Collapse outdated diffs regardless of discussion resolution |
| `resolve_outdated_diff_discussions` | boolean | no | Automatically resolve merge request diffs discussions on lines changed with a push |
| `container_registry_enabled` | boolean | no | Enable container registry for this project |
| `shared_runners_enabled` | boolean | no | Enable shared runners for this project |
| `visibility` | string | no | See [project visibility level](#project-visibility-level) |
......@@ -617,7 +617,7 @@
| `jobs_enabled` | boolean | no | Enable jobs for this project |
| `wiki_enabled` | boolean | no | Enable wiki for this project |
| `snippets_enabled` | boolean | no | Enable snippets for this project |
| `collapse_outdated_diff_comments` | boolean | no | Collapse outdated diffs regardless of discussion resolution |
| `resolve_outdated_diff_discussions` | boolean | no | Automatically resolve merge request diffs discussions on lines changed with a push |
| `container_registry_enabled` | boolean | no | Enable container registry for this project |
| `shared_runners_enabled` | boolean | no | Enable shared runners for this project |
| `visibility` | string | no | See [project visibility level](#project-visibility-level) |
......@@ -691,7 +691,7 @@
"jobs_enabled": true,
"wiki_enabled": true,
"snippets_enabled": false,
"collapse_outdated_diff_comments": false,
"resolve_outdated_diff_discussions": false,
"container_registry_enabled": false,
"created_at": "2013-09-30T13:46:02Z",
"last_activity_at": "2013-09-30T13:46:02Z",
......@@ -767,7 +767,7 @@
"jobs_enabled": true,
"wiki_enabled": true,
"snippets_enabled": false,
"collapse_outdated_diff_comments": false,
"resolve_outdated_diff_discussions": false,
"container_registry_enabled": false,
"created_at": "2013-09-30T13:46:02Z",
"last_activity_at": "2013-09-30T13:46:02Z",
......@@ -849,7 +849,7 @@
"jobs_enabled": true,
"wiki_enabled": true,
"snippets_enabled": false,
"collapse_outdated_diff_comments": false,
"resolve_outdated_diff_discussions": false,
"container_registry_enabled": false,
"created_at": "2013-09-30T13:46:02Z",
"last_activity_at": "2013-09-30T13:46:02Z",
......@@ -943,7 +943,7 @@
"jobs_enabled": true,
"wiki_enabled": true,
"snippets_enabled": false,
"collapse_outdated_diff_comments": false,
"resolve_outdated_diff_discussions": false,
"container_registry_enabled": false,
"created_at": "2013-09-30T13:46:02Z",
"last_activity_at": "2013-09-30T13:46:02Z",
......
......@@ -116,5 +116,5 @@
![Only allow merge if all the discussions are resolved message](img/only_allow_merge_if_all_discussions_are_resolved_msg.png)
### Automatically collapse outdated diff comments regardless of discussion resolution
### Automatically resolve merge request diffs discussions on lines changed with a push
......@@ -120,5 +120,4 @@
You can automatically collapse comments on outdated diffs regardless of whether
the discussion is resolved.
You can automatically resolve merge request diff discussions on lines modified with a new commit.
Navigate to your project's settings page, select the
......@@ -123,5 +122,5 @@
Navigate to your project's settings page, select the
**Collapse outdated diffs regardless of discussion resolution** check box and hit
**Automatically resolve merge request diffs discussions on lines changed with a push** check box and hit
**Save** for the changes to take effect.
......@@ -126,4 +125,4 @@
**Save** for the changes to take effect.
![Automatically collapse outdated diff comments regardless of discussion resolution](img/automatically_collapse_outdated_diff_comments_regardless_of_discussion_resolution.png)
![Automatically resolve merge request diffs discussions on lines changed with a push](img/automatically_resolve_outdated_diff_discussions.png)
......@@ -129,5 +128,5 @@
From now on, any discussions on an outdated diff will be collapsed by default.
From now on, any discussions on an outdated diff will be resolved by default.
## Threaded discussions
......
......@@ -119,7 +119,7 @@
expose :archived?, as: :archived
expose :visibility
expose :owner, using: Entities::UserBasic, unless: ->(project, options) { project.group }
expose :collapse_outdated_diff_comments
expose :resolve_outdated_diff_discussions
expose :container_registry_enabled
# Expose old field names with the new permissions methods to keep API compatible
......
......@@ -16,7 +16,7 @@
optional :jobs_enabled, type: Boolean, desc: 'Flag indication if jobs are enabled'
optional :snippets_enabled, type: Boolean, desc: 'Flag indication if snippets are enabled'
optional :shared_runners_enabled, type: Boolean, desc: 'Flag indication if shared runners are enabled for that project'
optional :collapse_outdated_diff_comments, type: Boolean, desc: 'Collapse outdated diffs regardless of discussion resolution'
optional :resolve_outdated_diff_discussions, type: Boolean, desc: 'Automatically resolve merge request diffs discussions on lines changed with a push'
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.'
......@@ -237,7 +237,7 @@
at_least_one_of_ce =
[
:jobs_enabled,
:collapse_outdated_diff_comments,
:resolve_outdated_diff_discussions,
:container_registry_enabled,
:default_branch,
:description,
......
......@@ -64,7 +64,7 @@
expose :owner, using: ::API::Entities::UserBasic, unless: ->(project, options) { project.group }
expose :name, :name_with_namespace
expose :path, :path_with_namespace
expose :collapse_outdated_diff_comments
expose :resolve_outdated_diff_discussions
expose :container_registry_enabled
# Expose old field names with the new permissions methods to keep API compatible
......
......@@ -18,7 +18,7 @@
optional :builds_enabled, type: Boolean, desc: 'Flag indication if builds are enabled'
optional :snippets_enabled, type: Boolean, desc: 'Flag indication if snippets are enabled'
optional :shared_runners_enabled, type: Boolean, desc: 'Flag indication if shared runners are enabled for that project'
optional :collapse_outdated_diff_comments, type: Boolean, desc: 'Collapse outdated diffs regardless of discussion resolution'
optional :resolve_outdated_diff_discussions, type: Boolean, desc: 'Automatically resolve merge request diffs discussions on lines changed with a push'
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 :public, type: Boolean, desc: 'Create a public project. The same as visibility_level = 20.'
......@@ -297,7 +297,7 @@
use :optional_params
at_least_one_of :name, :description, :issues_enabled, :merge_requests_enabled,
:wiki_enabled, :builds_enabled, :snippets_enabled,
:shared_runners_enabled, :collapse_outdated_diff_comments,
:shared_runners_enabled, :resolve_outdated_diff_discussions,
:container_registry_enabled, :lfs_enabled, :public, :visibility_level,
:public_builds, :request_access_enabled, :only_allow_merge_if_build_succeeds,
:only_allow_merge_if_all_discussions_are_resolved, :path,
......
require 'spec_helper'
feature 'Collapse outdated diff comments', js: true do
let(:merge_request) { create(:merge_request, importing: true) }
let(:project) { merge_request.source_project }
let!(:outdated_discussion) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: outdated_position).to_discussion }
let!(:active_discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion }
let(:outdated_position) do
Gitlab::Diff::Position.new(
old_path: "files/ruby/popen.rb",
new_path: "files/ruby/popen.rb",
old_line: nil,
new_line: 9,
diff_refs: outdated_diff_refs
)
end
let(:outdated_diff_refs) { project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e").diff_refs }
before do
sign_in(create(:admin))
end
context 'when project.collapse_outdated_diff_comments == true' do
before do
project.update_column(:collapse_outdated_diff_comments, true)
end
context 'with unresolved outdated discussions' do
it 'does not show outdated discussion' do
visit_merge_request(merge_request)
within(".discussion[data-discussion-id='#{outdated_discussion.id}']") do
expect(page).to have_css('.discussion-body .hide .js-toggle-content', visible: false)
end
end
end
context 'with unresolved active discussions' do
it 'shows active discussion' do
visit_merge_request(merge_request)
within(".discussion[data-discussion-id='#{active_discussion.id}']") do
expect(page).to have_css('.discussion-body .hide .js-toggle-content', visible: true)
end
end
end
end
context 'when project.collapse_outdated_diff_comments == false' do
before do
project.update_column(:collapse_outdated_diff_comments, false)
end
context 'with unresolved outdated discussions' do
it 'shows outdated discussion' do
visit_merge_request(merge_request)
within(".discussion[data-discussion-id='#{outdated_discussion.id}']") do
expect(page).to have_css('.discussion-body .hide .js-toggle-content', visible: true)
end
end
end
context 'with unresolved active discussions' do
it 'shows active discussion' do
visit_merge_request(merge_request)
within(".discussion[data-discussion-id='#{active_discussion.id}']") do
expect(page).to have_css('.discussion-body .hide .js-toggle-content', visible: true)
end
end
end
end
def visit_merge_request(merge_request)
visit project_merge_request_path(project, merge_request)
end
end
require 'spec_helper'
feature 'Resolve outdated diff discussions', js: true do
let(:merge_request) { create(:merge_request, importing: true) }
let(:project) { merge_request.source_project }
let!(:outdated_discussion) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: outdated_position).to_discussion }
let!(:active_discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion }
let(:outdated_position) do
Gitlab::Diff::Position.new(
old_path: "files/ruby/popen.rb",
new_path: "files/ruby/popen.rb",
old_line: nil,
new_line: 9,
diff_refs: outdated_diff_refs
)
end
let(:outdated_diff_refs) { project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e").diff_refs }
before do
sign_in(create(:admin))
end
context 'when project.resolve_outdated_diff_discussions == true' do
before do
project.update_column(:resolve_outdated_diff_discussions, true)
end
context 'with unresolved outdated discussions' do
it 'does not show outdated discussion' do
visit_merge_request(merge_request)
within(".discussion[data-discussion-id='#{outdated_discussion.id}']") do
expect(page).to have_css('.discussion-body .hide .js-toggle-content', visible: false)
expect(page).to have_content('Automatically resolved')
end
end
end
context 'with unresolved active discussions' do
it 'shows active discussion' do
visit_merge_request(merge_request)
within(".discussion[data-discussion-id='#{active_discussion.id}']") do
expect(page).to have_css('.discussion-body .hide .js-toggle-content', visible: true)
expect(page).not_to have_content('Automatically resolved')
end
end
end
end
context 'when project.resolve_outdated_diff_discussions == false' do
before do
project.update_column(:resolve_outdated_diff_discussions, false)
end
context 'with unresolved outdated discussions' do
it 'shows outdated discussion' do
visit_merge_request(merge_request)
within(".discussion[data-discussion-id='#{outdated_discussion.id}']") do
expect(page).to have_css('.discussion-body .hide .js-toggle-content', visible: true)
expect(page).not_to have_content('Automatically resolved')
end
end
end
context 'with unresolved active discussions' do
it 'shows active discussion' do
visit_merge_request(merge_request)
within(".discussion[data-discussion-id='#{active_discussion.id}']") do
expect(page).to have_css('.discussion-body .hide .js-toggle-content', visible: true)
expect(page).not_to have_content('Automatically resolved')
end
end
end
end
def visit_merge_request(merge_request)
visit project_merge_request_path(project, merge_request)
end
end
......@@ -414,7 +414,7 @@
jobs_enabled: false,
merge_requests_enabled: false,
wiki_enabled: false,
collapse_outdated_diff_comments: false,
resolve_outdated_diff_discussions: false,
only_allow_merge_if_pipeline_succeeds: false,
request_access_enabled: true,
only_allow_merge_if_all_discussions_are_resolved: false,
......@@ -478,6 +478,6 @@
expect(json_response['avatar_url']).to eq("http://localhost/uploads/-/system/project/avatar/#{project_id}/banana_sample.gif")
end
it 'sets a project as allowing outdated diff comments to collapse regardless of discussion resolution' do
project = attributes_for(:project, { collapse_outdated_diff_comments: false })
it 'sets a project as allowing outdated diff discussions to automatically resolve' do
project = attributes_for(:project, { resolve_outdated_diff_discussions: false })
post api('/projects', user), project
......@@ -483,4 +483,4 @@
post api('/projects', user), project
expect(json_response['collapse_outdated_diff_comments']).to be_falsey
expect(json_response['resolve_outdated_diff_discussions']).to be_falsey
end
......@@ -485,5 +485,5 @@
end
it 'sets a project as allowing outdated diff comments to collapse if collapse_outdated_diff_comments' do
project = attributes_for(:project, { collapse_outdated_diff_comments: true })
it 'sets a project as allowing outdated diff discussions to automatically resolve if resolve_outdated_diff_discussions' do
project = attributes_for(:project, { resolve_outdated_diff_discussions: true })
post api('/projects', user), project
......@@ -489,5 +489,5 @@
post api('/projects', user), project
expect(json_response['collapse_outdated_diff_comments']).to be_truthy
expect(json_response['resolve_outdated_diff_discussions']).to be_truthy
end
it 'sets a project as allowing merge even if build fails' do
......@@ -655,8 +655,8 @@
expect(json_response['visibility']).to eq('private')
end
it 'sets a project as allowing outdated diff comments to collapse regardless of discussion resolution' do
project = attributes_for(:project, { collapse_outdated_diff_comments: false })
it 'sets a project as allowing outdated diff discussions to automatically resolve' do
project = attributes_for(:project, { resolve_outdated_diff_discussions: false })
post api("/projects/user/#{user.id}", admin), project
......@@ -660,6 +660,6 @@
post api("/projects/user/#{user.id}", admin), project
expect(json_response['collapse_outdated_diff_comments']).to be_falsey
expect(json_response['resolve_outdated_diff_discussions']).to be_falsey
end
......@@ -664,7 +664,7 @@
end
it 'sets a project as allowing outdated diff comments to collapse only if collapse_outdated_diff_comments' do
project = attributes_for(:project, { collapse_outdated_diff_comments: true })
it 'sets a project as allowing outdated diff discussions to automatically resolve only if resolve_outdated_diff_discussions' do
project = attributes_for(:project, { resolve_outdated_diff_discussions: true })
post api("/projects/user/#{user.id}", admin), project
......@@ -668,7 +668,7 @@
post api("/projects/user/#{user.id}", admin), project
expect(json_response['collapse_outdated_diff_comments']).to be_truthy
expect(json_response['resolve_outdated_diff_discussions']).to be_truthy
end
it 'sets a project as allowing merge even if build fails' do
......@@ -761,7 +761,7 @@
expect(json_response['wiki_enabled']).to be_present
expect(json_response['jobs_enabled']).to be_present
expect(json_response['snippets_enabled']).to be_present
expect(json_response['collapse_outdated_diff_comments']).to eq(project.collapse_outdated_diff_comments)
expect(json_response['resolve_outdated_diff_discussions']).to eq(project.resolve_outdated_diff_discussions)
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
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment