From 6ef87a20832d1a2581cb85e60eda46f999c55a81 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 19 Jul 2017 16:03:50 +0200 Subject: 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. --- spec/services/boards/issues/list_service_spec.rb | 2 +- spec/services/boards/issues/move_service_spec.rb | 2 +- spec/services/delete_merged_branches_service_spec.rb | 2 +- spec/services/merge_requests/get_urls_service_spec.rb | 2 +- spec/services/merge_requests/reopen_service_spec.rb | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) (limited to 'spec/services') 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) -- cgit v1.2.1