Merge pull request #4592 from ulferts/fix/performance_on_list_cfs

Fix/performance on list cfs

frontend/app/components/api/api-v3/api-v3.config.ts

@@ -31,8 +31,13 @@ import {opApiModule} from '../../../angular-modules';
 function apiV3Config(apiV3, HalResource) {
   apiV3.addResponseInterceptor((data, operation, what) => {
     apiV3.addElementTransformer(what, HalResource.create);
+
     if (data) {
-      data._plain = angular.copy(data);
+      // lodash's clone seems to have better performance in our situation
+      // when compared to angular.clone
+      // see also:
+      // https://github.com/angular/angular.js/issues/11099
+      data._plain = _.clone(data, true);
 
       if (data._type === 'Collection') {
         const resp = [];

lib/api/v3/utilities/custom_field_injector.rb

@@ -144,7 +144,7 @@ module API
           when 'user'
             inject_user_schema(custom_field, customized)
           when 'list'
-            inject_list_schema(custom_field)
+            inject_list_schema(custom_field, customized)
           else
             inject_basic_schema(custom_field)
           end
@@ -184,8 +184,8 @@ module API
                                                 type: 'Version',
                                                 name_source: -> (*) { custom_field.name },
                                                 values_callback: -> (*) {
-                                                  customized.assignable_values(:version,
-                                                                               current_user)
+                                                  customized
+                                                    .assignable_custom_field_values(custom_field)
                                                 },
                                                 writable: true,
                                                 value_representer: Versions::VersionRepresenter,
@@ -213,13 +213,14 @@ module API
                                           }
         end
 
-        def inject_list_schema(custom_field)
+        def inject_list_schema(custom_field, customized)
           representer = StringObjects::StringObjectRepresenter
           @class.schema_with_allowed_collection property_name(custom_field.id),
                                                 type: 'StringObject',
                                                 name_source: -> (*) { custom_field.name },
                                                 values_callback: -> (*) {
-                                                  custom_field.possible_values
+                                                  customized
+                                                    .assignable_custom_field_values(custom_field)
                                                 },
                                                 value_representer: representer,
                                                 writable: true,

lib/api/v3/work_packages/schema/base_work_package_schema.rb

@@ -73,6 +73,10 @@ module API
             nil
           end
 
+          def assignable_custom_field_values(_custom_field)
+            nil
+          end
+
           def available_custom_fields
             []
           end

lib/api/v3/work_packages/schema/specific_work_package_schema.rb

@@ -62,6 +62,15 @@ module API
             end
           end
 
+          def assignable_custom_field_values(custom_field)
+            case custom_field.field_format
+            when 'list'
+              custom_field.possible_values
+            when 'version'
+              assignable_values(:version, nil)
+            end
+          end
+
           def available_custom_fields
             # we might have received a (currently) invalid work package
             return [] if project.nil? || type.nil?

spec/factories/custom_field_factory.rb

@@ -80,6 +80,22 @@ FactoryGirl.define do
       end
     end
 
+    factory :wp_custom_field, class: WorkPackageCustomField do
+      sequence(:name) do |n| "Work package custom field #{n}" end
+      type 'WorkPackageCustomField'
+
+      factory :list_wp_custom_field do
+        sequence(:name) do |n| "List work package custom field #{n}" end
+        field_format 'list'
+        possible_values ['1', '2', '3', '4', '5', '6', '7']
+      end
+
+      factory :version_wp_custom_field do
+        sequence(:name) do |n| "Version work package custom field #{n}" end
+        field_format 'version'
+      end
+    end
+
     factory :issue_custom_field, class: WorkPackageCustomField do
       sequence(:name) do |n| "Issue Custom Field #{n}" end
 

spec/lib/api/v3/utilities/custom_field_injector_spec.rb

@@ -57,14 +57,9 @@ describe ::API::V3::Utilities::CustomFieldInjector do
              defines_assignable_values?: true,
              available_custom_fields: [custom_field])
     }
-    let(:versions) { [] }
 
     subject { modified_class.new(schema, current_user: nil, form_embedded: true).to_json }
 
-    before do
-      allow(schema).to receive(:assignable_values).with(:version, anything).and_return(versions)
-    end
-
     describe 'basic custom field' do
       it_behaves_like 'has basic schema properties' do
         let(:path) { cf_path }
@@ -121,13 +116,18 @@ describe ::API::V3::Utilities::CustomFieldInjector do
 
     describe 'version custom field' do
       let(:custom_field) {
-        FactoryGirl.build(:custom_field,
-                          field_format: 'version',
+        FactoryGirl.build(:version_wp_custom_field,
                           is_required: true)
       }
+
       let(:assignable_versions) { FactoryGirl.build_list(:version, 3) }
 
       before do
+        allow(schema)
+          .to receive(:assignable_custom_field_values)
+          .with(custom_field)
+          .and_return(assignable_versions)
+
         allow(::API::V3::Versions::VersionRepresenter).to receive(:new).and_return(double)
       end
 
@@ -141,20 +141,28 @@ describe ::API::V3::Utilities::CustomFieldInjector do
 
       it_behaves_like 'links to allowed values directly' do
         let(:path) { cf_path }
-        let(:hrefs) { versions.map { |version| api_v3_paths.version version.id } }
+        let(:hrefs) { assignable_versions.map { |version| api_v3_paths.version version.id } }
       end
 
       it 'embeds allowed values' do
         # N.B. we do not use the stricter 'links to and embeds allowed values directly' helper
         # because this would not allow us to easily mock the VersionRepresenter away
-        is_expected.to have_json_size(versions.size).at_path("#{cf_path}/_embedded/allowedValues")
+        is_expected
+          .to have_json_size(assignable_versions.size)
+          .at_path("#{cf_path}/_embedded/allowedValues")
       end
     end
 
     describe 'list custom field' do
+      before do
+        allow(schema)
+          .to receive(:assignable_custom_field_values)
+          .with(custom_field)
+          .and_return(custom_field.possible_values)
+      end
+
       let(:custom_field) {
-        FactoryGirl.build(:custom_field,
-                          field_format: 'list',
+        FactoryGirl.build(:list_wp_custom_field,
                           is_required: true,
                           possible_values: values)
       }

spec/lib/api/v3/work_packages/schema/specific_work_package_schema_spec.rb

@@ -280,4 +280,23 @@ describe ::API::V3::WorkPackages::Schema::SpecificWorkPackageSchema do
       end
     end
   end
+
+  describe '#assignable_custom_field_values' do
+    let(:list_cf) { FactoryGirl.build_stubbed(:list_wp_custom_field) }
+    let(:version_cf) { FactoryGirl.build_stubbed(:version_wp_custom_field) }
+
+    it "is a list custom fields' possible values" do
+      expect(subject.assignable_custom_field_values(list_cf)).to eql list_cf.possible_values
+    end
+
+    it "is a version custom fields' project values" do
+      result = double('versions')
+
+      allow(work_package)
+        .to receive(:assignable_versions)
+        .and_return(result)
+
+      expect(subject.assignable_custom_field_values(version_cf)).to eql result
+    end
+  end
 end

spec/lib/api/v3/work_packages/schema/typed_work_package_schema_spec.rb

@@ -84,4 +84,17 @@ describe ::API::V3::WorkPackages::Schema::TypedWorkPackageSchema do
       is_expected.not_to be_milestone
     end
   end
+
+  describe '#assignable_custom_field_values' do
+    let(:list_cf) { FactoryGirl.build_stubbed(:list_wp_custom_field) }
+    let(:version_cf) { FactoryGirl.build_stubbed(:version_wp_custom_field) }
+
+    it 'is nil for a list cf' do
+      expect(subject.assignable_custom_field_values(list_cf)).to be_nil
+    end
+
+    it 'is nil for a version cf' do
+      expect(subject.assignable_custom_field_values(version_cf)).to be_nil
+    end
+  end
 end