Implement final review comments from @rymai.

1. Instantiate `ProtectedBranchesAccessSelect` from `dispatcher`

2. Use `can?(user, ...)` instead of `user.can?(...)`

3. Add `DOWNTIME` notes to all migrations added in !5081.

4. Add an explicit `down` method for migrations removing the
   `developers_can_push` and `developers_can_merge` columns, ensuring that
   the columns created (on rollback) have the appropriate defaults.

5. Remove duplicate CHANGELOG entries.

6. Blank lines after guard clauses.

6 files changed, 46 insertions, 2 deletions

diff --git a/db/migrate/20160705054938_add_protected_branches_push_access.rb b/db/migrate/20160705054938_add_protected_branches_push_access.rb
index 5c14d449e71..f27295524e1 100644
--- a/db/migrate/20160705054938_add_protected_branches_push_access.rb
+++ b/db/migrate/20160705054938_add_protected_branches_push_access.rb
@@ -2,6 +2,8 @@
 # for more information on how to write migrations for GitLab.
 
 class AddProtectedBranchesPushAccess < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :protected_branch_push_access_levels do |t|
       t.references :protected_branch, index: { name: "index_protected_branch_push_access" }, foreign_key: true, null: false
diff --git a/db/migrate/20160705054952_add_protected_branches_merge_access.rb b/db/migrate/20160705054952_add_protected_branches_merge_access.rb
index 789e3e04220..32adfa266cd 100644
--- a/db/migrate/20160705054952_add_protected_branches_merge_access.rb
+++ b/db/migrate/20160705054952_add_protected_branches_merge_access.rb
@@ -2,6 +2,8 @@
 # for more information on how to write migrations for GitLab.
 
 class AddProtectedBranchesMergeAccess < ActiveRecord::Migration
+  DOWNTIME = false
+
   def change
     create_table :protected_branch_merge_access_levels do |t|
       t.references :protected_branch, index: { name: "index_protected_branch_merge_access" }, foreign_key: true, null: false
diff --git a/db/migrate/20160705055254_move_from_developers_can_merge_to_protected_branches_merge_access.rb b/db/migrate/20160705055254_move_from_developers_can_merge_to_protected_branches_merge_access.rb
index c2b278ce673..fa93936ced7 100644
--- a/db/migrate/20160705055254_move_from_developers_can_merge_to_protected_branches_merge_access.rb
+++ b/db/migrate/20160705055254_move_from_developers_can_merge_to_protected_branches_merge_access.rb
@@ -2,6 +2,15 @@
 # for more information on how to write migrations for GitLab.
 
 class MoveFromDevelopersCanMergeToProtectedBranchesMergeAccess < ActiveRecord::Migration
+  DOWNTIME = true
+  DOWNTIME_REASON = <<-HEREDOC
+    We're creating a `merge_access_level` for each `protected_branch`. If a user creates a `protected_branch` while this
+    is running, we might be left with a `protected_branch` _without_ an associated `merge_access_level`. The `protected_branches`
+    table must not change while this is running, so downtime is required.
+
+    https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5081#note_13247410
+  HEREDOC
+
   def up
     execute <<-HEREDOC
       INSERT into protected_branch_merge_access_levels (protected_branch_id, access_level, created_at, updated_at)
diff --git a/db/migrate/20160705055308_move_from_developers_can_push_to_protected_branches_push_access.rb b/db/migrate/20160705055308_move_from_developers_can_push_to_protected_branches_push_access.rb
index 5bc70283f60..56f6159d1d8 100644
--- a/db/migrate/20160705055308_move_from_developers_can_push_to_protected_branches_push_access.rb
+++ b/db/migrate/20160705055308_move_from_developers_can_push_to_protected_branches_push_access.rb
@@ -2,6 +2,15 @@
 # for more information on how to write migrations for GitLab.
 
 class MoveFromDevelopersCanPushToProtectedBranchesPushAccess < ActiveRecord::Migration
+  DOWNTIME = true
+  DOWNTIME_REASON = <<-HEREDOC
+    We're creating a `push_access_level` for each `protected_branch`. If a user creates a `protected_branch` while this
+    is running, we might be left with a `protected_branch` _without_ an associated `push_access_level`. The `protected_branches`
+    table must not change while this is running, so downtime is required.
+
+    https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5081#note_13247410
+  HEREDOC
+
   def up
     execute <<-HEREDOC
       INSERT into protected_branch_push_access_levels (protected_branch_id, access_level, created_at, updated_at)
diff --git a/db/migrate/20160705055809_remove_developers_can_push_from_protected_branches.rb b/db/migrate/20160705055809_remove_developers_can_push_from_protected_branches.rb
index ad6ad43686d..f563f660ddf 100644
--- a/db/migrate/20160705055809_remove_developers_can_push_from_protected_branches.rb
+++ b/db/migrate/20160705055809_remove_developers_can_push_from_protected_branches.rb
@@ -2,7 +2,18 @@
 # for more information on how to write migrations for GitLab.
 
 class RemoveDevelopersCanPushFromProtectedBranches < ActiveRecord::Migration
-  def change
+  include Gitlab::Database::MigrationHelpers
+
+  # This is only required for `#down`
+  disable_ddl_transaction!
+
+  DOWNTIME = false
+
+  def up
     remove_column :protected_branches, :developers_can_push, :boolean
   end
+
+  def down
+    add_column_with_default(:protected_branches, :developers_can_push, :boolean, default: false, null: false)
+  end
 end
diff --git a/db/migrate/20160705055813_remove_developers_can_merge_from_protected_branches.rb b/db/migrate/20160705055813_remove_developers_can_merge_from_protected_branches.rb
index 084914e423a..aa71e06d36e 100644
--- a/db/migrate/20160705055813_remove_developers_can_merge_from_protected_branches.rb
+++ b/db/migrate/20160705055813_remove_developers_can_merge_from_protected_branches.rb
@@ -2,7 +2,18 @@
 # for more information on how to write migrations for GitLab.
 
 class RemoveDevelopersCanMergeFromProtectedBranches < ActiveRecord::Migration
-  def change
+  include Gitlab::Database::MigrationHelpers
+
+  # This is only required for `#down`
+  disable_ddl_transaction!
+
+  DOWNTIME = false
+
+  def up
     remove_column :protected_branches, :developers_can_merge, :boolean
   end
+
+  def down
+    add_column_with_default(:protected_branches, :developers_can_merge, :boolean, default: false, null: false)
+  end
 end