Merge issuable "reopened" state into "opened"merge-issuable-reopened-into-opened-state

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.

10 files changed, 10 insertions, 26 deletions

diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb
index f1fd1fd7f73..a16695cb7c1 100644
--- a/spec/factories/issues.rb
+++ b/spec/factories/issues.rb
@@ -16,12 +16,8 @@ FactoryGirl.define do
       state :closed
     end
 
-    trait :reopened do
-      state :reopened
-    end
-
     factory :closed_issue, traits: [:closed]
-    factory :reopened_issue, traits: [:reopened]
+    factory :reopened_issue, traits: [:opened]
 
     factory :labeled_issue do
       transient do
diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb
index 253a025af48..1bc530d06db 100644
--- a/spec/factories/merge_requests.rb
+++ b/spec/factories/merge_requests.rb
@@ -44,10 +44,6 @@ FactoryGirl.define do
       state :opened
     end
 
-    trait :reopened do
-      state :reopened
-    end
-
     trait :locked do
       state :locked
     end
@@ -74,7 +70,7 @@ FactoryGirl.define do
 
     factory :merged_merge_request, traits: [:merged]
     factory :closed_merge_request, traits: [:closed]
-    factory :reopened_merge_request, traits: [:reopened]
+    factory :reopened_merge_request, traits: [:opened]
     factory :merge_request_with_diffs, traits: [:with_diffs]
     factory :merge_request_with_diff_notes do
       after(:create) do |mr|
diff --git a/spec/lib/banzai/filter/issuable_state_filter_spec.rb b/spec/lib/banzai/filter/issuable_state_filter_spec.rb
index 7cf2f4282f8..bc7cae1df8d 100644
--- a/spec/lib/banzai/filter/issuable_state_filter_spec.rb
+++ b/spec/lib/banzai/filter/issuable_state_filter_spec.rb
@@ -107,14 +107,6 @@ describe Banzai::Filter::IssuableStateFilter do
       expect(doc.css('a').last.text).to eq(issue.to_reference)
     end
 
-    it 'ignores reopened issue references' do
-      issue = create_issue(:reopened)
-      link = create_link(issue.to_reference, issue: issue.id, reference_type: 'issue')
-      doc = filter(link, context)
-
-      expect(doc.css('a').last.text).to eq(issue.to_reference)
-    end
-
     it 'appends state to closed issue references' do
       link = create_link(closed_issue.to_reference, issue: closed_issue.id, reference_type: 'issue')
       doc = filter(link, context)
@@ -139,7 +131,7 @@ describe Banzai::Filter::IssuableStateFilter do
     end
 
     it 'ignores reopened merge request references' do
-      merge_request = create_merge_request(:reopened)
+      merge_request = create_merge_request(:opened)
 
       link = create_link(
         merge_request.to_reference,
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 33cea02153e..2c44be4e447 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -1259,7 +1259,7 @@ describe API::Issues do
       put api("/projects/#{project.id}/issues/#{closed_issue.iid}", user), state_event: 'reopen'
 
       expect(response).to have_http_status(200)
-      expect(json_response['state']).to eq 'reopened'
+      expect(json_response['state']).to eq 'opened'
     end
 
     context 'when an admin or owner makes the request' do
diff --git a/spec/requests/api/v3/issues_spec.rb b/spec/requests/api/v3/issues_spec.rb
index cc81922697a..4dff09b6df8 100644
--- a/spec/requests/api/v3/issues_spec.rb
+++ b/spec/requests/api/v3/issues_spec.rb
@@ -1114,7 +1114,7 @@ describe API::V3::Issues do
       put v3_api("/projects/#{project.id}/issues/#{closed_issue.id}", user), state_event: 'reopen'
 
       expect(response).to have_http_status(200)
-      expect(json_response['state']).to eq 'reopened'
+      expect(json_response['state']).to eq 'opened'
     end
 
     context 'when an admin or owner makes the request' do
diff --git a/spec/services/boards/issues/list_service_spec.rb b/spec/services/boards/issues/list_service_spec.rb
index 2c293088097..b1b5d807a78 100644
--- a/spec/services/boards/issues/list_service_spec.rb
+++ b/spec/services/boards/issues/list_service_spec.rb
@@ -20,7 +20,7 @@ describe Boards::Issues::ListService do
 
     let!(:opened_issue1) { create(:labeled_issue, project: project, labels: [bug]) }
     let!(:opened_issue2) { create(:labeled_issue, project: project, labels: [p2]) }
-    let!(:reopened_issue1) { create(:issue, :reopened, project: project) }
+    let!(:reopened_issue1) { create(:issue, :opened, project: project) }
 
     let!(:list1_issue1) { create(:labeled_issue, project: project, labels: [p2, development]) }
     let!(:list1_issue2) { create(:labeled_issue, project: project, labels: [development]) }
diff --git a/spec/services/boards/issues/move_service_spec.rb b/spec/services/boards/issues/move_service_spec.rb
index 7dd1a601700..15a32350ae2 100644
--- a/spec/services/boards/issues/move_service_spec.rb
+++ b/spec/services/boards/issues/move_service_spec.rb
@@ -73,7 +73,7 @@ describe Boards::Issues::MoveService do
         issue.reload
 
         expect(issue.labels).to contain_exactly(bug, testing)
-        expect(issue).to be_reopened
+        expect(issue).to be_opened
       end
     end
 
diff --git a/spec/services/delete_merged_branches_service_spec.rb b/spec/services/delete_merged_branches_service_spec.rb
index 954c5d3ab73..4b872d667cf 100644
--- a/spec/services/delete_merged_branches_service_spec.rb
+++ b/spec/services/delete_merged_branches_service_spec.rb
@@ -43,7 +43,7 @@ describe DeleteMergedBranchesService do
     context 'open merge requests' do
       it 'does not delete branches from open merge requests' do
         fork_link = create(:forked_project_link, forked_from_project: project)
-        create(:merge_request, :reopened, source_project: project, target_project: project, source_branch: 'branch-merged', target_branch: 'master')
+        create(:merge_request, :opened, source_project: project, target_project: project, source_branch: 'branch-merged', target_branch: 'master')
         create(:merge_request, :opened, source_project: fork_link.forked_to_project, target_project: project, target_branch: 'improve/awesome', source_branch: 'master')
 
         service.execute
diff --git a/spec/services/merge_requests/get_urls_service_spec.rb b/spec/services/merge_requests/get_urls_service_spec.rb
index 4a7d8ab4c6c..672d86e4028 100644
--- a/spec/services/merge_requests/get_urls_service_spec.rb
+++ b/spec/services/merge_requests/get_urls_service_spec.rb
@@ -78,7 +78,7 @@ describe MergeRequests::GetUrlsService do
     end
 
     context 'pushing to existing branch and merge request is reopened' do
-      let!(:merge_request) { create(:merge_request, :reopened, source_project: project, source_branch: source_branch) }
+      let!(:merge_request) { create(:merge_request, :opened, source_project: project, source_branch: source_branch) }
       let(:changes) { existing_branch_changes }
       it_behaves_like 'show_merge_request_url'
     end
diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb
index ce1e9f2ff45..f02af0c582e 100644
--- a/spec/services/merge_requests/reopen_service_spec.rb
+++ b/spec/services/merge_requests/reopen_service_spec.rb
@@ -28,7 +28,7 @@ describe MergeRequests::ReopenService do
       end
 
       it { expect(merge_request).to be_valid }
-      it { expect(merge_request).to be_reopened }
+      it { expect(merge_request).to be_opened }
 
       it 'executes hooks with reopen action' do
         expect(service).to have_received(:execute_hooks)