always have current status as an option for the new status (#4423)

The current status will always be available as an option when the work package is queried about what status it can be changed into.

app/models/work_package.rb

@@ -436,10 +436,11 @@ class WorkPackage < ActiveRecord::Base
       author == user,
       assigned_to_id_changed? ? assigned_to_id_was == user.id : assigned_to_id == user.id
     )
-    statuses << status unless statuses.empty?
     statuses << Status.default if include_default
-    statuses = statuses.uniq.sort
-    blocked? ? statuses.reject(&:is_closed?) : statuses
+    statuses.reject!(&:is_closed?) if blocked?
+    statuses << status
+
+    statuses.uniq.sort
   end
 
   # >>> issues.rb >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

spec/models/status_spec.rb

@@ -28,40 +28,13 @@
 
 require 'spec_helper'
 
-describe WorkPackage, type: :model do
-  let(:stub_work_package) { FactoryGirl.build_stubbed(:work_package) }
-  let(:stub_version) { FactoryGirl.build_stubbed(:version) }
-  let(:stub_project) { FactoryGirl.build_stubbed(:project) }
-  let(:work_package) { FactoryGirl.create(:work_package) }
-  let(:user) { FactoryGirl.create(:user) }
-
-  let(:type) { FactoryGirl.create(:type_standard) }
-  let(:project) { FactoryGirl.create(:project, types: [type]) }
-  let(:status) { FactoryGirl.create(:status) }
-  let(:priority) { FactoryGirl.create(:priority) }
-  let(:work_package) {
-    WorkPackage.new.tap do |w|
-      w.attributes = { project_id: project.id,
-                       type_id: type.id,
-                       author_id: user.id,
-                       status_id: status.id,
-                       priority: priority,
-                       subject: 'test_create',
-                       description: 'WorkPackage#create',
-                       estimated_hours: '1:30' }
-    end
-  }
-  describe '#new_statuses_allowed_to' do
+describe Status, type: :model do
+  describe '#find_new_statuses_allowed_to and #new_statuses_allowed_to' do
     let(:role) { FactoryGirl.create(:role) }
     let(:type) { FactoryGirl.create(:type) }
     let(:user) { FactoryGirl.create(:user) }
-    let(:other_user) { FactoryGirl.create(:user) }
     let(:statuses) { (1..5).map { |_i| FactoryGirl.create(:status) } }
-    let(:priority) { FactoryGirl.create :priority, is_default: true }
     let(:status) { statuses[0] }
-    let(:project) do
-      FactoryGirl.create(:project, types: [type]).tap { |p| p.add_member(user, role).save }
-    end
     let(:workflow_a) {
       FactoryGirl.create(:workflow, role_id: role.id,
                                     type_id: type.id,
@@ -96,8 +69,11 @@ describe WorkPackage, type: :model do
     }
     let(:workflows) { [workflow_a, workflow_b, workflow_c, workflow_d] }
 
-    it 'should respect workflows w/o author and w/o assignee' do
+    before do
       workflows
+    end
+
+    it 'should respect workflows w/o author and w/o assignee' do
       expect(status.new_statuses_allowed_to([role], type, false, false))
         .to match_array([statuses[1]])
       expect(status.find_new_statuses_allowed_to([role], type, false, false))
@@ -105,7 +81,6 @@ describe WorkPackage, type: :model do
     end
 
     it 'should respect workflows w/ author and w/o assignee' do
-      workflows
       expect(status.new_statuses_allowed_to([role], type, true, false))
         .to match_array([statuses[1], statuses[2]])
       expect(status.find_new_statuses_allowed_to([role], type, true, false))
@@ -113,7 +88,6 @@ describe WorkPackage, type: :model do
     end
 
     it 'should respect workflows w/o author and w/ assignee' do
-      workflows
       expect(status.new_statuses_allowed_to([role], type, false, true))
         .to match_array([statuses[1], statuses[3]])
       expect(status.find_new_statuses_allowed_to([role], type, false, true))
@@ -121,57 +95,10 @@ describe WorkPackage, type: :model do
     end
 
     it 'should respect workflows w/ author and w/ assignee' do
-      workflows
       expect(status.new_statuses_allowed_to([role], type, true, true))
         .to match_array([statuses[1], statuses[2], statuses[3], statuses[4]])
       expect(status.find_new_statuses_allowed_to([role], type, true, true))
         .to match_array([statuses[1], statuses[2], statuses[3], statuses[4]])
     end
-
-    it 'should respect workflows w/o author and w/o assignee on work packages' do
-      workflows
-      work_package = WorkPackage.create(type_id: type.id,
-                                        status: status,
-                                        priority: priority,
-                                        project: project)
-      expect(work_package.new_statuses_allowed_to(user)).to match_array([statuses[0], statuses[1]])
-    end
-
-    it 'should respect workflows w/ author and w/o assignee on work packages' do
-      workflows
-      work_package = WorkPackage.create(type_id: type.id,
-                                        status: status,
-                                        priority: priority,
-                                        project: project,
-                                        author: user)
-      expect(work_package.new_statuses_allowed_to(user))
-        .to match_array([statuses[0], statuses[1], statuses[2]])
-    end
-
-    it 'should respect workflows w/o author and w/ assignee on work packages' do
-      workflows
-      work_package = WorkPackage.create(type_id: type.id,
-                                        status: status,
-                                        subject: 'test',
-                                        priority: priority,
-                                        project: project,
-                                        assigned_to: user,
-                                        author: other_user)
-      expect(work_package.new_statuses_allowed_to(user))
-        .to match_array([statuses[0], statuses[1], statuses[3]])
-    end
-
-    it 'should respect workflows w/ author and w/ assignee on work packages' do
-      workflows
-      work_package = WorkPackage.create(type_id: type.id,
-                                        status: status,
-                                        subject: 'test',
-                                        priority: priority,
-                                        project: project,
-                                        author: user,
-                                        assigned_to: user)
-      expect(work_package.new_statuses_allowed_to(user))
-        .to match_array([statuses[0], statuses[1], statuses[2], statuses[3], statuses[4]])
-    end
   end
 end

spec/models/work_package/work_package_status_spec.rb

@@ -28,123 +28,225 @@
 
 require 'spec_helper'
 
-describe WorkPackage, type: :model do
-  describe 'status' do
-    let(:status) { FactoryGirl.create(:status) }
+describe WorkPackage, 'status', type: :model do
+  let(:status) { FactoryGirl.create(:status) }
+  let!(:work_package) {
+    FactoryGirl.create(:work_package,
+                       status: status)
+  }
+
+  it 'can read planning_elements w/ the help of the has_many association' do
+    expect(WorkPackage.where(status_id: status.id).count).to eq(1)
+    expect(WorkPackage.where(status_id: status.id).first).to eq(work_package)
+  end
+
+  describe 'transition' do
+    let(:user) { FactoryGirl.create(:user) }
+    let(:type) { FactoryGirl.create(:type) }
+    let(:project) {
+      FactoryGirl.create(:project,
+                         types: [type])
+    }
+    let(:role) {
+      FactoryGirl.create(:role,
+                         permissions: [:edit_work_packages])
+    }
+    let(:invalid_role) {
+      FactoryGirl.create(:role,
+                         permissions: [:edit_work_packages])
+    }
+    let!(:member) {
+      FactoryGirl.create(:member,
+                         project: project,
+                         principal: user,
+                         roles: [role])
+    }
+    let(:status_2) { FactoryGirl.create(:status) }
     let!(:work_package) {
       FactoryGirl.create(:work_package,
+                         project_id: project.id,
+                         type_id: type.id,
                          status_id: status.id)
     }
+    let(:valid_user_workflow) {
+      FactoryGirl.create(:workflow,
+                         type_id: type.id,
+                         old_status: status,
+                         new_status: status_2,
+                         role: role)
+    }
+    let(:invalid_user_workflow) {
+      FactoryGirl.create(:workflow,
+                         type_id: type.id,
+                         old_status: status,
+                         new_status: status_2,
+                         role: invalid_role)
+    }
 
-    it 'can read planning_elements w/ the help of the has_many association' do
-      expect(WorkPackage.where(status_id: status.id).count).to eq(1)
-      expect(WorkPackage.where(status_id: status.id).first).to eq(work_package)
-    end
+    shared_examples_for 'work package status transition' do
+      describe 'valid' do
+        before do
+          valid_user_workflow
 
-    describe 'transition' do
-      let(:user) { FactoryGirl.create(:user) }
-      let(:type) { FactoryGirl.create(:type) }
-      let(:project) {
-        FactoryGirl.create(:project,
-                           types: [type])
-      }
-      let(:role) {
-        FactoryGirl.create(:role,
-                           permissions: [:edit_work_packages])
-      }
-      let(:invalid_role) {
-        FactoryGirl.create(:role,
-                           permissions: [:edit_work_packages])
-      }
-      let!(:member) {
-        FactoryGirl.create(:member,
-                           project: project,
-                           principal: user,
-                           roles: [role])
-      }
-      let(:status_2) { FactoryGirl.create(:status) }
-      let!(:work_package) {
-        FactoryGirl.create(:work_package,
-                           project_id: project.id,
-                           type_id: type.id,
-                           status_id: status.id)
-      }
-      let(:valid_user_workflow) {
-        FactoryGirl.create(:workflow,
-                           type_id: type.id,
-                           old_status: status,
-                           new_status: status_2,
-                           role: role)
-      }
-      let(:invalid_user_workflow) {
-        FactoryGirl.create(:workflow,
-                           type_id: type.id,
-                           old_status: status,
-                           new_status: status_2,
-                           role: invalid_role)
-      }
-
-      shared_examples_for 'work package status transition' do
-        describe 'valid' do
-          before do
-            valid_user_workflow
-
-            work_package.status = status_2
-          end
-
-          it { expect(work_package.save).to be_truthy }
+          work_package.status = status_2
         end
 
-        describe 'invalid' do
-          before do
-            invalid_user_workflow
+        it { expect(work_package.save).to be_truthy }
+      end
 
-            work_package.status = status_2
-          end
+      describe 'invalid' do
+        before do
+          invalid_user_workflow
 
-          it { expect(work_package.save).to eq(invalid_result) }
+          work_package.status = status_2
         end
 
-        describe 'non-existing' do
-          before do work_package.status = status_2 end
+        it { expect(work_package.save).to eq(invalid_result) }
+      end
 
-          it { expect(work_package.save).to be_falsey }
-        end
+      describe 'non-existing' do
+        before do work_package.status = status_2 end
+
+        it { expect(work_package.save).to be_falsey }
       end
+    end
 
-      describe 'non-admin user' do
-        before do allow(User).to receive(:current).and_return user end
+    describe 'non-admin user' do
+      before do allow(User).to receive(:current).and_return user end
 
-        it_behaves_like 'work package status transition' do
-          let(:invalid_result) { false }
-        end
+      it_behaves_like 'work package status transition' do
+        let(:invalid_result) { false }
       end
+    end
 
-      describe 'admin user' do
-        let(:admin) { FactoryGirl.create(:admin) }
+    describe 'admin user' do
+      let(:admin) { FactoryGirl.create(:admin) }
 
-        before do allow(User).to receive(:current).and_return admin end
+      before do allow(User).to receive(:current).and_return admin end
 
-        it_behaves_like 'work package status transition' do
-          let(:invalid_result) { true }
-        end
+      it_behaves_like 'work package status transition' do
+        let(:invalid_result) { true }
       end
+    end
 
-      describe 'transition to non-existing status' do
-        before do
-          work_package.status_id = -1
-          work_package.valid?
-        end
+    describe 'transition to non-existing status' do
+      before do
+        work_package.status_id = -1
+        work_package.valid?
+      end
+
+      it 'should not have the error on the :status_id field' do
+        expect(work_package.errors).not_to have_key(:status_id)
+      end
+
+      it 'has an error' do
+        expect(work_package.errors[:status].count).to eql(1)
+      end
+
+      it 'has a blank error on the .status field' do
+        expect(work_package.errors[:status].first)
+          .to eql(I18n.t('activerecord.errors.messages.blank'))
+      end
+    end
+  end
+
+  describe '#new_statuses_allowed_to' do
+    let(:role) { FactoryGirl.build_stubbed(:role) }
+    let(:type) { FactoryGirl.build_stubbed(:type) }
+    let(:user) { FactoryGirl.build_stubbed(:user) }
+    let(:current_status) { FactoryGirl.build_stubbed(:status) }
+    let(:work_package) do
+      FactoryGirl.build_stubbed(:work_package,
+                                status: current_status)
+    end
+    let(:other_status) { FactoryGirl.build_stubbed(:status) }
+    let(:default_status) do
+      status = FactoryGirl.build_stubbed(:status)
+
+      allow(Status)
+        .to receive(:default)
+        .and_return(status)
+
+      status
+    end
+
+    let(:roles) { [role] }
+
+    before do
+      expect(user)
+        .to receive(:roles_for_project)
+        .with(work_package.project)
+        .and_return(roles)
+
+      allow(current_status)
+        .to receive(:find_new_statuses_allowed_to)
+        .and_return([])
+    end
+
+    shared_examples_for 'new_statuses_allowed_to' do
+      it 'returns the statuses' do
+        expect(work_package.new_statuses_allowed_to(user))
+          .to match_array([current_status, other_status])
+      end
+
+      it 'adds the default status when the parameter is set accordingly' do
+        default_status
+
+        expect(work_package.new_statuses_allowed_to(user, true))
+          .to match_array([current_status, other_status, default_status])
+      end
 
-        it { expect(work_package.errors).not_to have_key(:status_id) }
+      it 'removes closed statuses if blocked' do
+        other_status.is_closed = true
 
-        it { expect(work_package.errors[:status].count).to eql(1) }
+        allow(work_package)
+          .to receive(:blocked?)
+          .and_return(true)
 
-        it {
-          expect(work_package.errors[:status].first)
-            .to eql(I18n.t('activerecord.errors.messages.blank'))
-        }
+        expect(work_package.new_statuses_allowed_to(user))
+          .to match_array([current_status])
       end
     end
+
+    context 'with somebody else asking' do
+      before do
+        allow(current_status)
+          .to receive(:find_new_statuses_allowed_to)
+          .with(roles, work_package.type, false, false)
+          .and_return([other_status])
+      end
+
+      it_behaves_like 'new_statuses_allowed_to'
+    end
+
+    context 'with the author asking' do
+      before do
+        work_package.author = user
+
+        allow(current_status)
+          .to receive(:find_new_statuses_allowed_to)
+          .with(roles, work_package.type, true, false)
+          .and_return([other_status])
+      end
+
+      it_behaves_like 'new_statuses_allowed_to'
+    end
+
+    context 'with the assignee asking' do
+      before do
+        work_package.assigned_to_id = user.id
+
+        allow(work_package)
+          .to receive(:assigned_to_id_changed?)
+          .and_return false
+
+        allow(current_status)
+          .to receive(:find_new_statuses_allowed_to)
+          .with(roles, work_package.type, false, true)
+          .and_return([other_status])
+      end
+
+      it_behaves_like 'new_statuses_allowed_to'
+    end
   end
 end