Add functionality to collapse outdated diff comments regardless of discussion resolution

2017-08-28 17:38:06 -04:00 · 2017-08-28 17:38:06 -04:00 · ef4b3a39bc
parent 446c7fc6f1
commit ef4b3a39bc
16 changed files with 74 additions and 4 deletions
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@ -323,6 +323,7 @@ class ProjectsController < Projects::ApplicationController
      :build_allow_git_fetch,
      :build_coverage_regex,
      :build_timeout_in_minutes,
+      :collapse_outdated_diff_comments,
      :container_registry_enabled,
      :default_branch,
      :description,
--- a/app/models/diff_discussion.rb
+++ b/app/models/diff_discussion.rb
@ -18,6 +18,14 @@ 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?
--- a/app/models/project.rb
+++ b/app/models/project.rb
@ -37,6 +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 :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 }
--- a/app/views/projects/_merge_request_merge_settings.html.haml
+++ b/app/views/projects/_merge_request_merge_settings.html.haml
@ -17,3 +17,7 @@
    = 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
--- a/changelogs/unreleased/36994-toggle-for-automatically-collapsing-outdated-diff-comments.yml
+++ b/changelogs/unreleased/36994-toggle-for-automatically-collapsing-outdated-diff-comments.yml
@ -0,0 +1,6 @@
+---
+title: Add repository toggle for automatically collapsing outdated diff comments regardless
+  of discussion resolution
+merge_request:
+author:
+type: added
--- a/db/migrate/20170825154015_collapse_outdated_diff_comments.rb
+++ b/db/migrate/20170825154015_collapse_outdated_diff_comments.rb
@ -0,0 +1,18 @@
+# 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
--- a/db/schema.rb
+++ b/db/schema.rb
@ -1202,6 +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 "container_registry_enabled"
    t.boolean "only_allow_merge_if_pipeline_succeeds", default: false, null: false
    t.boolean "has_external_issue_tracker"
--- a/doc/api/projects.md
+++ b/doc/api/projects.md
@ -70,6 +70,7 @@ Parameters:
    "jobs_enabled": true,
    "wiki_enabled": true,
    "snippets_enabled": false,
+    "collapse_outdated_diff_comments": false,
    "container_registry_enabled": false,
    "created_at": "2013-09-30T13:46:02Z",
    "last_activity_at": "2013-09-30T13:46:02Z",
@ -137,6 +138,7 @@ Parameters:
    "jobs_enabled": true,
    "wiki_enabled": true,
    "snippets_enabled": false,
+    "collapse_outdated_diff_comments": false,
    "container_registry_enabled": false,
    "created_at": "2013-09-30T13:46:02Z",
    "last_activity_at": "2013-09-30T13:46:02Z",
@ -246,6 +248,7 @@ Parameters:
    "jobs_enabled": true,
    "wiki_enabled": true,
    "snippets_enabled": false,
+    "collapse_outdated_diff_comments": false,
    "container_registry_enabled": false,
    "created_at": "2013-09-30T13:46:02Z",
    "last_activity_at": "2013-09-30T13:46:02Z",
@ -313,6 +316,7 @@ Parameters:
    "jobs_enabled": true,
    "wiki_enabled": true,
    "snippets_enabled": false,
+    "collapse_outdated_diff_comments": false,
    "container_registry_enabled": false,
    "created_at": "2013-09-30T13:46:02Z",
    "last_activity_at": "2013-09-30T13:46:02Z",
@ -411,6 +415,7 @@ Parameters:
  "jobs_enabled": true,
  "wiki_enabled": true,
  "snippets_enabled": false,
+  "collapse_outdated_diff_comments": false,
  "container_registry_enabled": false,
  "created_at": "2013-09-30T13:46:02Z",
  "last_activity_at": "2013-09-30T13:46:02Z",
@ -537,6 +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 |
 | `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) |
@ -574,6 +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 |
 | `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) |
@ -610,6 +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 |
 | `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) |
@ -683,6 +691,7 @@ Example response:
  "jobs_enabled": true,
  "wiki_enabled": true,
  "snippets_enabled": false,
+  "collapse_outdated_diff_comments": false,
  "container_registry_enabled": false,
  "created_at": "2013-09-30T13:46:02Z",
  "last_activity_at": "2013-09-30T13:46:02Z",
@ -758,6 +767,7 @@ Example response:
  "jobs_enabled": true,
  "wiki_enabled": true,
  "snippets_enabled": false,
+  "collapse_outdated_diff_comments": false,
  "container_registry_enabled": false,
  "created_at": "2013-09-30T13:46:02Z",
  "last_activity_at": "2013-09-30T13:46:02Z",
@ -839,6 +849,7 @@ Example response:
  "jobs_enabled": true,
  "wiki_enabled": true,
  "snippets_enabled": false,
+  "collapse_outdated_diff_comments": false,
  "container_registry_enabled": false,
  "created_at": "2013-09-30T13:46:02Z",
  "last_activity_at": "2013-09-30T13:46:02Z",
@ -932,6 +943,7 @@ Example response:
  "jobs_enabled": true,
  "wiki_enabled": true,
  "snippets_enabled": false,
+  "collapse_outdated_diff_comments": false,
  "container_registry_enabled": false,
  "created_at": "2013-09-30T13:46:02Z",
  "last_activity_at": "2013-09-30T13:46:02Z",
--- 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
--- a/doc/user/discussions/index.md
+++ b/doc/user/discussions/index.md
@ -116,6 +116,19 @@ 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
+
+You can automatically collapse comments on outdated diffs regardless of whether
+the discussion is resolved.
+
+Navigate to your project's settings page, select the
+**Collapse outdated diffs regardless of discussion resolution** 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)
+
+From now on, any discussions on an outdated diff will be collapsed by default. 
+
 ## Threaded discussions

 > [Introduced][ce-7527] in GitLab 9.1.
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@ -119,6 +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 :container_registry_enabled

      # Expose old field names with the new permissions methods to keep API compatible
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@ -16,6 +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 :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.'
@ -236,6 +237,7 @@ module API
        at_least_one_of_ce =
          [
            :jobs_enabled,
+            :collapse_outdated_diff_comments,
            :container_registry_enabled,
            :default_branch,
            :description,
--- a/lib/api/v3/entities.rb
+++ b/lib/api/v3/entities.rb
@ -64,6 +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 :container_registry_enabled

        # Expose old field names with the new permissions methods to keep API compatible
--- a/lib/api/v3/projects.rb
+++ b/lib/api/v3/projects.rb
@ -18,6 +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 :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.'
@ -296,9 +297,9 @@ 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, :container_registry_enabled,
-            :lfs_enabled, :public, :visibility_level, :public_builds,
-            :request_access_enabled, :only_allow_merge_if_build_succeeds,
+            :shared_runners_enabled, :collapse_outdated_diff_comments,
+            :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,
            :default_branch
        end
--- a/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb
+++ b/spec/lib/gitlab/background_migration/migrate_events_to_push_event_payloads_spec.rb
@ -214,7 +214,7 @@ end
 # The background migration relies on a temporary table, hence we're migrating
 # to a specific version of the database where said table is still present.
 #
-describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads, :migration, schema: 20170608152748 do
+describe Gitlab::BackgroundMigration::MigrateEventsToPushEventPayloads, :migration, schema: 20170825154015 do
  let(:migration) { described_class.new }
  let(:project) { create(:project_empty_repo) }
  let(:author) { create(:user) }
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@ -398,6 +398,7 @@ Project:
 - public_builds
 - last_repository_check_failed
 - last_repository_check_at
+- collapse_outdated_diff_comments
 - container_registry_enabled
 - only_allow_merge_if_pipeline_succeeds
 - has_external_issue_tracker