From a3f76b76a4b8db85c6fa557a5e801dcea7195735 Mon Sep 17 00:00:00 2001 From: Ashley Dumaine Date: Fri, 1 Sep 2017 18:39:22 -0400 Subject: change collapse to resolve and comments to discussions --- app/controllers/projects_controller.rb | 2 +- app/models/diff_discussion.rb | 8 --- app/models/project.rb | 2 +- app/views/discussions/_headline.html.haml | 21 ++++-- .../_merge_request_merge_settings.html.haml | 8 +-- ...matically-collapsing-outdated-diff-comments.yml | 3 +- ...170825154015_collapse_outdated_diff_comments.rb | 18 ----- ...0825154015_resolve_outdated_diff_discussions.rb | 17 +++++ db/schema.rb | 2 +- doc/api/projects.md | 24 +++---- ...omments_regardless_of_discussion_resolution.png | Bin 129633 -> 0 bytes ...matically_resolve_outdated_diff_discussions.png | Bin 0 -> 107568 bytes doc/user/discussions/index.md | 11 ++- lib/api/entities.rb | 2 +- lib/api/projects.rb | 4 +- lib/api/v3/entities.rb | 2 +- lib/api/v3/projects.rb | 4 +- .../collapse_outdated_diff_comments.rb | 76 -------------------- .../resolve_outdated_diff_discussions.rb | 80 +++++++++++++++++++++ spec/requests/api/projects_spec.rb | 28 ++++---- spec/requests/api/v3/projects_spec.rb | 28 ++++---- 21 files changed, 171 insertions(+), 169 deletions(-) delete mode 100644 db/migrate/20170825154015_collapse_outdated_diff_comments.rb create mode 100644 db/migrate/20170825154015_resolve_outdated_diff_discussions.rb delete mode 100644 doc/user/discussions/img/automatically_collapse_outdated_diff_comments_regardless_of_discussion_resolution.png create mode 100644 doc/user/discussions/img/automatically_resolve_outdated_diff_discussions.png delete mode 100644 spec/features/merge_requests/collapse_outdated_diff_comments.rb create mode 100644 spec/features/merge_requests/resolve_outdated_diff_discussions.rb diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 3cc61b5e682..b13034d3333 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -323,7 +323,7 @@ class ProjectsController < Projects::ApplicationController :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, diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb index 2b41de9cbcf..07c4846e2ac 100644 --- a/app/models/diff_discussion.rb +++ b/app/models/diff_discussion.rb @@ -18,14 +18,6 @@ class DiffDiscussion < Discussion 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? diff --git a/app/models/project.rb b/app/models/project.rb index 3b822f39fc0..fdd516ec2ae 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -37,7 +37,7 @@ class Project < ActiveRecord::Base 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 } diff --git a/app/views/discussions/_headline.html.haml b/app/views/discussions/_headline.html.haml index 25e90924413..a9fe805bfa2 100644 --- a/app/views/discussions/_headline.html.haml +++ b/app/views/discussions/_headline.html.haml @@ -1,10 +1,19 @@ - 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 diff --git a/app/views/projects/_merge_request_merge_settings.html.haml b/app/views/projects/_merge_request_merge_settings.html.haml index d040e8da65b..c1cb3b07d8e 100644 --- a/app/views/projects/_merge_request_merge_settings.html.haml +++ b/app/views/projects/_merge_request_merge_settings.html.haml @@ -13,11 +13,11 @@ = form.label :only_allow_merge_if_all_discussions_are_resolved do = 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 - .checkbox - = form.label :collapse_outdated_diff_comments do - = form.check_box :collapse_outdated_diff_comments - %strong Collapse outdated diffs regardless of discussion resolution diff --git a/changelogs/unreleased/36994-toggle-for-automatically-collapsing-outdated-diff-comments.yml b/changelogs/unreleased/36994-toggle-for-automatically-collapsing-outdated-diff-comments.yml index 9a3f3c49931..c6aedbbc21a 100644 --- a/changelogs/unreleased/36994-toggle-for-automatically-collapsing-outdated-diff-comments.yml +++ b/changelogs/unreleased/36994-toggle-for-automatically-collapsing-outdated-diff-comments.yml @@ -1,6 +1,5 @@ --- -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 diff --git a/db/migrate/20170825154015_collapse_outdated_diff_comments.rb b/db/migrate/20170825154015_collapse_outdated_diff_comments.rb deleted file mode 100644 index cbfdd3ba2c2..00000000000 --- a/db/migrate/20170825154015_collapse_outdated_diff_comments.rb +++ /dev/null @@ -1,18 +0,0 @@ -# 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 diff --git a/db/migrate/20170825154015_resolve_outdated_diff_discussions.rb b/db/migrate/20170825154015_resolve_outdated_diff_discussions.rb new file mode 100644 index 00000000000..8ee474e908e --- /dev/null +++ b/db/migrate/20170825154015_resolve_outdated_diff_discussions.rb @@ -0,0 +1,17 @@ +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 diff --git a/db/schema.rb b/db/schema.rb index 02b365a8743..c25ca40b906 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1202,7 +1202,7 @@ ActiveRecord::Schema.define(version: 20170901071411) do 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" diff --git a/doc/api/projects.md b/doc/api/projects.md index 345b88daa79..2d973547d14 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -70,7 +70,7 @@ Parameters: "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 @@ Parameters: "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 @@ Parameters: "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 @@ Parameters: "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 @@ Parameters: "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 @@ Parameters: | `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 @@ Parameters: | `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 @@ Parameters: | `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 @@ Example response: "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 @@ Example response: "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 @@ Example response: "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 @@ Example response: "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", diff --git a/doc/user/discussions/img/automatically_collapse_outdated_diff_comments_regardless_of_discussion_resolution.png b/doc/user/discussions/img/automatically_collapse_outdated_diff_comments_regardless_of_discussion_resolution.png deleted file mode 100644 index 4c273637bfc..00000000000 Binary files a/doc/user/discussions/img/automatically_collapse_outdated_diff_comments_regardless_of_discussion_resolution.png and /dev/null differ diff --git a/doc/user/discussions/img/automatically_resolve_outdated_diff_discussions.png b/doc/user/discussions/img/automatically_resolve_outdated_diff_discussions.png new file mode 100644 index 00000000000..500c24ee175 Binary files /dev/null and b/doc/user/discussions/img/automatically_resolve_outdated_diff_discussions.png differ diff --git a/doc/user/discussions/index.md b/doc/user/discussions/index.md index 5771b420fdf..b2264c15d63 100644 --- a/doc/user/discussions/index.md +++ b/doc/user/discussions/index.md @@ -116,18 +116,17 @@ are resolved. ![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 -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 -**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. -![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) -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 diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 2799fec9cb6..9114b69606b 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -119,7 +119,7 @@ module API 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 diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 558eace80f3..7dc19788462 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -16,7 +16,7 @@ module API 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 @@ module API at_least_one_of_ce = [ :jobs_enabled, - :collapse_outdated_diff_comments, + :resolve_outdated_diff_discussions, :container_registry_enabled, :default_branch, :description, diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index c00df7d00de..ac47a713966 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -64,7 +64,7 @@ module API 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 diff --git a/lib/api/v3/projects.rb b/lib/api/v3/projects.rb index 2a8906587ce..74df246bdfe 100644 --- a/lib/api/v3/projects.rb +++ b/lib/api/v3/projects.rb @@ -18,7 +18,7 @@ module API 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 @@ module API 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, diff --git a/spec/features/merge_requests/collapse_outdated_diff_comments.rb b/spec/features/merge_requests/collapse_outdated_diff_comments.rb deleted file mode 100644 index baca34026ea..00000000000 --- a/spec/features/merge_requests/collapse_outdated_diff_comments.rb +++ /dev/null @@ -1,76 +0,0 @@ -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 diff --git a/spec/features/merge_requests/resolve_outdated_diff_discussions.rb b/spec/features/merge_requests/resolve_outdated_diff_discussions.rb new file mode 100644 index 00000000000..fd9b2c95210 --- /dev/null +++ b/spec/features/merge_requests/resolve_outdated_diff_discussions.rb @@ -0,0 +1,80 @@ +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 diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 5e3fdbd468e..1e71634b5db 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -414,7 +414,7 @@ describe API::Projects do 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,16 +478,16 @@ describe API::Projects do 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 - expect(json_response['collapse_outdated_diff_comments']).to be_falsey + expect(json_response['resolve_outdated_diff_discussions']).to be_falsey 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 - 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,20 +655,20 @@ describe API::Projects do 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 - expect(json_response['collapse_outdated_diff_comments']).to be_falsey + expect(json_response['resolve_outdated_diff_discussions']).to be_falsey 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 - 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 @@ describe API::Projects do 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 diff --git a/spec/requests/api/v3/projects_spec.rb b/spec/requests/api/v3/projects_spec.rb index bbe56c972bd..f8dd5f960e8 100644 --- a/spec/requests/api/v3/projects_spec.rb +++ b/spec/requests/api/v3/projects_spec.rb @@ -394,7 +394,7 @@ describe API::V3::Projects do issues_enabled: false, merge_requests_enabled: false, wiki_enabled: false, - collapse_outdated_diff_comments: false, + resolve_outdated_diff_discussions: false, only_allow_merge_if_build_succeeds: false, request_access_enabled: true, only_allow_merge_if_all_discussions_are_resolved: false @@ -456,16 +456,16 @@ describe API::V3::Projects do expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::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 v3_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 - 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 v3_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 @@ -612,16 +612,16 @@ describe API::V3::Projects do expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::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 v3_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 - 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 v3_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 @@ -712,7 +712,7 @@ describe API::V3::Projects do expect(json_response['wiki_enabled']).to be_present expect(json_response['builds_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 -- cgit v1.2.3