Merge branch 'ee-merge-issuable-reopened-into-opened-state' into 'master'

EE: Merge issuable "reopened" state into "opened"

See merge request !2492

app/assets/javascripts/issuable/related_issues/components/issue_token.vue

@@ -53,7 +53,7 @@ export default {
       return this.state && this.state.length > 0;
     },
     isOpen() {
-      return this.state === 'opened' || this.state === 'reopened';
+      return this.state === 'opened';
     },
     isClosed() {
       return this.state === 'closed';

app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js

@@ -67,7 +67,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;

app/finders/issuable_finder.rb

@@ -85,7 +85,6 @@ class IssuableFinder
     end
 
     counts[:all] = counts.values.sum
-    counts[:opened] += counts[:reopened]
 
     counts.with_indifferent_access
   end

app/models/burndown.rb

 class Burndown
-  Issue = Struct.new(:closed_at, :weight, :state)
+  class Issue
+    attr_reader :closed_at, :weight, :state
+
+    def initialize(closed_at, weight, state)
+      @closed_at = closed_at
+      @weight = weight
+      @state = state
+    end
+
+    def reopened?
+      @state == 'opened' && @closed_at.present?
+    end
+  end
 
   attr_reader :start_date, :due_date, :end_date, :issues_count, :issues_weight, :accurate, :legacy_data
   alias_method :accurate?, :accurate
@@ -12,8 +24,8 @@ class Burndown
     @end_date = @milestone.due_date
     @end_date = Date.today if @end_date.present? && @end_date > Date.today
 
-    @accurate = milestone_closed_issues.all?(&:closed_at)
-    @legacy_data = milestone_closed_issues.any? && milestone_closed_issues.none?(&:closed_at)
+    @accurate = milestone_issues.all?(&:closed_at)
+    @legacy_data = milestone_issues.any? && milestone_issues.none?(&:closed_at)
 
     @issues_count, @issues_weight = milestone.issues.reorder(nil).pluck('COUNT(*), COALESCE(SUM(weight), 0)').first
   end
@@ -59,21 +71,21 @@ class Burndown
     current_date = date.to_date
 
     closed =
-      milestone_closed_issues.select do |issue|
+      milestone_issues.select do |issue|
         (issue.closed_at&.to_date || start_date) == current_date
       end
 
-    reopened = closed.select { |issue| issue.state == 'reopened' }
+    reopened = closed.select(&:reopened?)
 
     [closed, reopened]
   end
 
-  def milestone_closed_issues
-    @milestone_closed_issues ||=
+  def milestone_issues
+    @milestone_issues ||=
       @milestone.issues
-        .where("state IN ('reopened', 'closed')")
-        .order("closed_at ASC")
+        .where("state = 'closed' OR (state = 'opened' AND closed_at IS NOT NULL)")
+        .reorder("closed_at ASC")
         .pluck("closed_at, weight, state")
-        .map {|attrs| ::Burndown::Issue.new(*attrs) }
+        .map {|attrs| Issue.new(*attrs) }
   end
 end

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 :order_milestone_due_desc, -> { outer_join_milestone.reorder('milestones.due_date IS NULL ASC, milestones.due_date DESC, milestones.id DESC') }
     scope :order_milestone_due_asc, -> { outer_join_milestone.reorder('milestones.due_date IS NULL ASC, milestones.due_date ASC, milestones.id ASC') }
@@ -241,7 +240,7 @@ module Issuable
   end
 
   def open?
-    opened? || reopened?
+    opened?
   end
 
   def user_notes_count

app/models/issue.rb

@@ -73,15 +73,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|

app/models/merge_request.rb

@@ -45,23 +45,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|
@@ -75,7 +75,6 @@ class MergeRequest < ActiveRecord::Base
     end
 
     state :opened
-    state :reopened
     state :closed
     state :merged
     state :locked
@@ -372,7 +371,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?

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

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

app/services/merge_requests/base_service.rb

@@ -2,8 +2,8 @@ module MergeRequests
   class BaseService < ::IssuableBaseService
     prepend EE::MergeRequests::BaseService
 
-    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)
@@ -46,7 +46,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)

app/services/merge_requests/refresh_service.rb

@@ -82,7 +82,7 @@ module MergeRequests
 
     # Note: Closed merge requests also need approvals reset.
     def reset_approvals_for_merge_requests
-      merge_requests = merge_requests_for(@branch_name, mr_states: [:opened, :reopened, :closed])
+      merge_requests = merge_requests_for(@branch_name, mr_states: [:opened, :closed])
 
       merge_requests.each do |merge_request|
         target_project = merge_request.target_project

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)

changelogs/unreleased/merge-issuable-reopened-into-opened-state.yml

+---
+title: Merge issuable "reopened" state into "opened"
+merge_request:
+author:

db/post_migrate/20170719150301_merge_issuable_reopened_into_opened_state.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class MergeIssuableReopenedIntoOpenedState < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  class Issue < ActiveRecord::Base
+    self.table_name = 'issues'
+
+    include EachBatch
+  end
+
+  class MergeRequest < ActiveRecord::Base
+    self.table_name = 'merge_requests'
+
+    include EachBatch
+  end
+
+  def up
+    [Issue, MergeRequest].each do |model|
+      say "Changing #{model.table_name}.state from 'reopened' to 'opened'"
+
+      model.where(state: 'reopened').each_batch do |batch|
+        batch.update_all(state: 'opened')
+      end
+    end
+  end
+end

spec/factories/issues.rb

@@ -17,12 +17,8 @@ FactoryGirl.define do
       closed_at Time.now
     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

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
@@ -80,7 +76,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_approver, traits: [:with_approver]
     factory :merge_request_with_diff_notes do

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,

spec/models/burndown_spec.rb

@@ -63,14 +63,14 @@ describe Burndown do
     expect(burndown).to be_accurate
   end
 
-  context "when all closed and reopened issues does not have closed_at" do
+  context "when all closed issues does not have closed_at" do
     before do
       milestone.issues.update_all(closed_at: nil)
     end
 
     it "considers closed_at as milestone start date" do
       expect(subject).to eq([
-        ["2017-03-01", 15, 30],
+        ["2017-03-01", 27, 54],
         ["2017-03-02", 27, 54],
         ["2017-03-03", 27, 54],
         ["2017-03-04", 27, 54],
@@ -85,7 +85,7 @@ describe Burndown do
     end
   end
 
-  context "when one or more closed or reopened issues does not have closed_at" do
+  context "when one or more closed issues does not have closed_at" do
     before do
       milestone.issues.closed.first.update(closed_at: nil)
     end

spec/requests/api/issues_spec.rb

@@ -1228,7 +1228,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

spec/requests/api/v3/issues_spec.rb

@@ -1116,7 +1116,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

spec/services/boards/issues/list_service_spec.rb

@@ -23,7 +23,7 @@ describe Boards::Issues::ListService do
 
     let!(:opened_issue1) { create(:labeled_issue, project: project, milestone: m1, title: 'Issue 1', labels: [bug]) }
     let!(:opened_issue2) { create(:labeled_issue, project: project, milestone: m2, title: 'Issue 2', labels: [p2]) }
-    let!(:reopened_issue1) { create(:issue, :reopened, project: project, title: 'Issue 3' ) }
+    let!(:reopened_issue1) { create(:issue, :opened, project: project, title: 'Issue 3' ) }
 
     let!(:list1_issue1) { create(:labeled_issue, project: project, milestone: m1, labels: [p2, development]) }
     let!(:list1_issue2) { create(:labeled_issue, project: project, milestone: m2, labels: [development]) }

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
 

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

spec/services/issues/export_csv_service_spec.rb

@@ -35,7 +35,7 @@ describe Issues::ExportCsvService do
       issue.update!(milestone: milestone,
                     assignees: [user],
                     description: 'Issue with details',
-                    state: :reopened,
+                    state: :opened,
                     due_date: DateTime.new(2014, 3, 2),
                     created_at: DateTime.new(2015, 4, 3, 2, 1, 0),
                     updated_at: DateTime.new(2016, 5, 4, 3, 2, 1),

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

spec/services/merge_requests/refresh_service_spec.rb

@@ -86,7 +86,7 @@ describe MergeRequests::RefreshService do
       let(:refresh_service) { service.new(@project, @user) }
 
       before do
-        @merge_request.update(state: :reopened)
+        @merge_request.update(state: :opened)
 
         allow(refresh_service).to receive(:execute_hooks)
         refresh_service.execute(@oldrev, @newrev, 'refs/heads/master')

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)