Merge issuable "reopened" state into "opened"

Having two states that essentially mean the same thing is very much like
having a boolean "true" and boolean "mostly-true": it's rather silly.
This commit merges the "reopened" state into the "opened" state while
taking care of system notes still showing messages along the lines of
"Alice reopened this issue".

A big benefit from having only two states (opened and closed) is that
indexing and querying becomes simpler and more performant. For example,
to get all the opened queries we no longer have to query both states:

    SELECT *
    FROM issues
    WHERE project_id = 2
    AND state IN ('opened', 'reopened');

Instead we can query a single state directly, which can be much faster:

    SELECT *
    FROM issues
    WHERE project_id = 2
    AND state = 'opened';

Further, only having two states makes indexing easier as we will only
ever filter (and thus scan an index) using a single value. Partial
indexes could help but aren't supported on MySQL, complicating the
development process and not being helpful for MySQL.

9 files changed, 19 insertions, 23 deletions

diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
index 72a13108404..fddafb0ddfa 100644
--- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
+++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
@@ -65,7 +65,7 @@ export default class MergeRequestStore {
     this.mergeCheckPath = data.merge_check_path;
     this.mergeActionsContentPath = data.commit_change_content_path;
     this.isRemovingSourceBranch = this.isRemovingSourceBranch || false;
-    this.isOpen = data.state === 'opened' || data.state === 'reopened' || false;
+    this.isOpen = data.state === 'opened';
     this.hasMergeableDiscussionsState = data.mergeable_discussions_state === false;
     this.canRemoveSourceBranch = currentUser.can_remove_source_branch || false;
     this.canMerge = !!data.merge_path;
diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 6fe17a2b99d..08a843ada97 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -81,7 +81,6 @@ class IssuableFinder
     end
 
     counts[:all] = counts.values.sum
-    counts[:opened] += counts[:reopened]
 
     counts
   end
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 13fe9d09c69..935ffe343ff 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -71,9 +71,8 @@ module Issuable
     scope :of_projects, ->(ids) { where(project_id: ids) }
     scope :of_milestones, ->(ids) { where(milestone_id: ids) }
     scope :with_milestone, ->(title) { left_joins_milestones.where(milestones: { title: title }) }
-    scope :opened, -> { with_state(:opened, :reopened) }
+    scope :opened, -> { with_state(:opened) }
     scope :only_opened, -> { with_state(:opened) }
-    scope :only_reopened, -> { with_state(:reopened) }
     scope :closed, -> { with_state(:closed) }
 
     scope :left_joins_milestones,    -> { joins("LEFT OUTER JOIN milestones ON #{table_name}.milestone_id = milestones.id") }
@@ -234,7 +233,7 @@ module Issuable
   end
 
   def open?
-    opened? || reopened?
+    opened?
   end
 
   def user_notes_count
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 400bb55d2f0..1c948c8957e 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -62,15 +62,14 @@ class Issue < ActiveRecord::Base
 
   state_machine :state, initial: :opened do
     event :close do
-      transition [:reopened, :opened] => :closed
+      transition [:opened] => :closed
     end
 
     event :reopen do
-      transition closed: :reopened
+      transition closed: :opened
     end
 
     state :opened
-    state :reopened
     state :closed
 
     before_transition any => :closed do |issue|
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index a910099b4c1..81e0776e79c 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -42,23 +42,23 @@ class MergeRequest < ActiveRecord::Base
 
   state_machine :state, initial: :opened do
     event :close do
-      transition [:reopened, :opened] => :closed
+      transition [:opened] => :closed
     end
 
     event :mark_as_merged do
-      transition [:reopened, :opened, :locked] => :merged
+      transition [:opened, :locked] => :merged
     end
 
     event :reopen do
-      transition closed: :reopened
+      transition closed: :opened
     end
 
     event :lock_mr do
-      transition [:reopened, :opened] => :locked
+      transition [:opened] => :locked
     end
 
     event :unlock_mr do
-      transition locked: :reopened
+      transition locked: :opened
     end
 
     after_transition any => :locked do |merge_request, transition|
@@ -72,7 +72,6 @@ class MergeRequest < ActiveRecord::Base
     end
 
     state :opened
-    state :reopened
     state :closed
     state :merged
     state :locked
@@ -368,7 +367,7 @@ class MergeRequest < ActiveRecord::Base
       errors.add :branch_conflict, "You can not use same project/branch for source and target"
     end
 
-    if opened? || reopened?
+    if opened?
       similar_mrs = self.target_project.merge_requests.where(source_branch: source_branch, target_branch: target_branch, source_project_id: source_project.try(:id)).opened
       similar_mrs = similar_mrs.where('id not in (?)', self.id) if self.id
       if similar_mrs.any?
diff --git a/app/models/project_services/drone_ci_service.rb b/app/models/project_services/drone_ci_service.rb
index f6cade9c290..c93f1632652 100644
--- a/app/models/project_services/drone_ci_service.rb
+++ b/app/models/project_services/drone_ci_service.rb
@@ -114,7 +114,7 @@ class DroneCiService < CiService
   end
 
   def merge_request_valid?(data)
-    %w(opened reopened).include?(data[:object_attributes][:state]) &&
+    data[:object_attributes][:state] == 'opened' &&
       data[:object_attributes][:merge_status] == 'unchecked'
   end
 end
diff --git a/app/services/issues/reopen_service.rb b/app/services/issues/reopen_service.rb
index 73b2e85cba3..35de4337b15 100644
--- a/app/services/issues/reopen_service.rb
+++ b/app/services/issues/reopen_service.rb
@@ -5,7 +5,7 @@ module Issues
 
       if issue.reopen
         event_service.reopen_issue(issue, current_user)
-        create_note(issue)
+        create_note(issue, 'reopened')
         notification_service.reopen_issue(issue, current_user)
         execute_hooks(issue, 'reopen')
         invalidate_cache_counts(issue, users: issue.assignees)
@@ -16,8 +16,8 @@ module Issues
 
     private
 
-    def create_note(issue)
-      SystemNoteService.change_status(issue, issue.project, current_user, issue.state, nil)
+    def create_note(issue, state = issue.state)
+      SystemNoteService.change_status(issue, issue.project, current_user, state, nil)
     end
   end
 end
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index 3542a41ac83..35ccff26262 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/app/services/merge_requests/base_service.rb
@@ -1,7 +1,7 @@
 module MergeRequests
   class BaseService < ::IssuableBaseService
-    def create_note(merge_request)
-      SystemNoteService.change_status(merge_request, merge_request.target_project, current_user, merge_request.state, nil)
+    def create_note(merge_request, state = merge_request.state)
+      SystemNoteService.change_status(merge_request, merge_request.target_project, current_user, state, nil)
     end
 
     def create_title_change_note(issuable, old_title)
@@ -44,7 +44,7 @@ module MergeRequests
     end
 
     # Returns all origin and fork merge requests from `@project` satisfying passed arguments.
-    def merge_requests_for(source_branch, mr_states: [:opened, :reopened])
+    def merge_requests_for(source_branch, mr_states: [:opened])
       MergeRequest
         .with_state(mr_states)
         .where(source_branch: source_branch, source_project_id: @project.id)
diff --git a/app/services/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb
index 52f6d511f98..b9c65be36ec 100644
--- a/app/services/merge_requests/reopen_service.rb
+++ b/app/services/merge_requests/reopen_service.rb
@@ -5,7 +5,7 @@ module MergeRequests
 
       if merge_request.reopen
         event_service.reopen_mr(merge_request, current_user)
-        create_note(merge_request)
+        create_note(merge_request, 'reopened')
         notification_service.reopen_mr(merge_request, current_user)
         execute_hooks(merge_request, 'reopen')
         merge_request.reload_diff(current_user)