reuse version calculation on eager loaded projects

Eager loading projects assigns the same object instance to each work package having the same project_id. When the first project evaluates the assignable versions, the result can be memoized to be used by other work package having the same project

app/models/project.rb

@@ -595,6 +595,19 @@ class Project < ActiveRecord::Base
     end
   end
 
+  # Returns all versions a work package can be assigned to.  Opposed to
+  # #shared_versions this returns an array of Versions, not a scope.
+  #
+  # The main benefit is in scenarios where work packages' projects are eager
+  # loaded.  Because eager loading the project e.g. via
+  # WorkPackage.includes(:project).where(type: 5) will assign the same instance
+  # (same object_id) for every work package having the same project this will
+  # reduce the number of db queries when performing operations including the
+  # project's versions.
+  def assignable_versions
+    @all_shared_versions ||= shared_versions.open.to_a
+  end
+
   # Returns a hash of project users grouped by role
   def users_by_role
     members.includes(:user, :roles).inject({}) do |h, m|

app/models/work_package.rb

@@ -399,7 +399,10 @@ class WorkPackage < ActiveRecord::Base
   #   * the version it was already assigned to
   #     (to make sure, that you can still update closed tickets)
   def assignable_versions
-    @assignable_versions ||= (project.shared_versions.open + [(fixed_version_id_changed? ? Version.find_by(id: fixed_version_id_was) : fixed_version)]).compact.uniq.sort
+    @assignable_versions ||= begin
+      current_version = fixed_version_id_changed? ? Version.find_by(id: fixed_version_id_was) : fixed_version
+      (project.assignable_versions + [current_version]).compact.uniq.sort
+    end
   end
 
   def kind

spec/models/work_package_spec.rb

@@ -224,17 +224,11 @@ describe WorkPackage, type: :model do
   end
 
   describe '#assignable_versions' do
+    let(:stub_version2) { FactoryGirl.build_stubbed(:version) }
     def stub_shared_versions(v = nil)
       versions = v ? [v] : []
 
-      # open seems to be defined on the array's singleton class
-      # as such it seems not possible to stub it
-      # achieving the same here
-      versions.define_singleton_method :open do
-        self
-      end
-
-      allow(stub_work_package.project).to receive(:shared_versions).and_return(versions)
+      allow(stub_work_package.project).to receive(:assignable_versions).and_return(versions)
     end
 
     it "should return all the project's shared versions" do
@@ -243,11 +237,24 @@ describe WorkPackage, type: :model do
       expect(stub_work_package.assignable_versions).to eq([stub_version])
     end
 
-    it 'should return the current fixed_version' do
+    it 'should return the former fixed_version if the version changed' do
+      stub_shared_versions
+
+      stub_work_package.fixed_version = stub_version2
+
+      allow(stub_work_package).to receive(:fixed_version_id_changed?).and_return true
+      allow(stub_work_package).to receive(:fixed_version_id_was).and_return(stub_version.id)
+      allow(Version).to receive(:find_by).with(id: stub_version.id).and_return(stub_version)
+
+      expect(stub_work_package.assignable_versions).to eq([stub_version])
+    end
+
+    it 'should return the current fixed_version if the versiondid not change' do
       stub_shared_versions
 
-      allow(stub_work_package).to receive(:fixed_version_id_was).and_return(5)
-      allow(Version).to receive(:find_by).with(id: 5).and_return(stub_version)
+      stub_work_package.fixed_version = stub_version
+
+      allow(stub_work_package).to receive(:fixed_version_id_changed?).and_return false
 
       expect(stub_work_package.assignable_versions).to eq([stub_version])
     end
@@ -700,6 +707,9 @@ describe WorkPackage, type: :model do
     }
 
     before do
+      version_1
+      version_2
+      project.reload
       work_package_1
       work_package_2
     end

spec_legacy/unit/project_spec.rb

@@ -1074,8 +1074,8 @@ describe Project, type: :model do
 
       it 'should return 100 if the version has only closed issues' do
         v1 = FactoryGirl.create(:version, project: @project)
-        FactoryGirl.create(:work_package, project: @project, status: Status.find_by_name('Closed'), fixed_version: v1)
         v2 = FactoryGirl.create(:version, project: @project)
+        FactoryGirl.create(:work_package, project: @project, status: Status.find_by_name('Closed'), fixed_version: v1)
         FactoryGirl.create(:work_package, project: @project, status: Status.find_by_name('Closed'), fixed_version: v2)
 
         assert_equal 100, @project.completed_percent
@@ -1083,8 +1083,8 @@ describe Project, type: :model do
 
       it 'should return the averaged completed percent of the versions (not weighted)' do
         v1 = FactoryGirl.create(:version, project: @project)
-        FactoryGirl.create(:work_package, project: @project, status: Status.find_by_name('New'), estimated_hours: 10, done_ratio: 50, fixed_version: v1)
         v2 = FactoryGirl.create(:version, project: @project)
+        FactoryGirl.create(:work_package, project: @project, status: Status.find_by_name('New'), estimated_hours: 10, done_ratio: 50, fixed_version: v1)
         FactoryGirl.create(:work_package, project: @project, status: Status.find_by_name('New'), estimated_hours: 10, done_ratio: 50, fixed_version: v2)
 
         assert_equal 50, @project.completed_percent