turn attribute group into dedicated object

app/helpers/types_helper.rb

@@ -31,19 +31,67 @@ module ::TypesHelper
   def icon_for_type(type)
     return unless type
 
-    if type.is_milestone?
+    css_class = if type.is_milestone?
-      css_class = 'timelines-milestone'
+                  'timelines-milestone'
-    else
+                else
-      css_class = 'timelines-phase'
+                  'timelines-phase'
-    end
+                end
-    if type.color.present?
+
-      color = type.color.hexcode
+    color = if type.color.present?
-    else
+              type.color.hexcode
-      color = '#CCC'
+            else
-    end
+              '#CCC'
+            end
 
     content_tag(:span, ' ',
                 class: css_class,
                 style: "background-color: #{color}")
   end
+
+  ##
+  # Collect active and inactive form configuration groups for editing.
+  def form_configuration_groups(type)
+    available = type.work_package_attributes
+    # First we create a complete list of all attributes.
+    # Later we will remove those that are members of an attribute group.
+    # This way attributes that were created after the las group definitions
+    # will fall back into the inactives group.
+    inactive = available.clone
+
+    active_form = get_active_groups(type, available, inactive)
+    inactive_form = inactive
+                    .map { |key, attribute| attr_form_map(key, attribute) }
+                    .sort_by { |attr| attr[:translation] }
+
+    {
+      actives: active_form,
+      inactives: inactive_form
+    }
+  end
+
+  private
+
+  ##
+  # Collect active attributes from the current form configuration.
+  # Using the available attributes from +work_package_attributes+,
+  # determines which attributes are not used
+  def get_active_groups(type, available, inactive)
+    type.non_query_attribute_groups.map do |group|
+      extended_attributes =
+        group.attributes
+             .select { |key| inactive.delete(key) }
+             .map! { |key| attr_form_map(key, available[key]) }
+
+      [group, extended_attributes]
+    end
+  end
+
+  def attr_form_map(key, represented)
+    {
+      key: key,
+      is_cf: CustomField.custom_field_attribute?(key),
+      is_required: represented[:required] && !represented[:has_default],
+      translation: Type.translated_attribute_name(key, represented)
+    }
+  end
 end

app/models/custom_field.rb

@@ -211,6 +211,10 @@ class CustomField < ActiveRecord::Base
     end
   end
 
+  def self.custom_field_attribute?(attribute_name)
+    attribute_name.to_s =~ /custom_field_\d+/
+  end
+
   # to move in project_custom_field
   def self.for_all(options = {})
     where(is_for_all: true)

app/models/type/attribute_group.rb

@@ -0,0 +1,41 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+class Type::AttributeGroup < Type::FormGroup
+  def members
+    attributes.sort
+  end
+
+  def active_members(project)
+    members.select do |prop|
+      type.passes_attribute_constraint?(prop, project: project)
+    end
+  end
+end

app/models/type/attribute_groups.rb

@@ -1,4 +1,5 @@
 #-- encoding: UTF-8
+
 #-- copyright
 # OpenProject is a project management system.
 # Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
@@ -80,24 +81,15 @@ module Type::AttributeGroups
     end
   end
 
-  ##
-  # Translate the given attribute group if its internal
-  # (== if it's a symbol)
-  def translated_attribute_group(groupkey)
-    if groupkey.is_a? Symbol
-      I18n.t(default_groups[groupkey])
-    else
-      groupkey
-    end
-  end
-
   ##
   # Read the serialized attribute groups, if customized.
   # Otherwise, return +default_attribute_groups+
   def attribute_groups
     groups = custom_attribute_groups || default_attribute_groups
 
-    groups + [[:children, [default_children_query]]]
+    groups = to_attribute_group_class(groups)
+
+    groups + [::Type::QueryGroup.new(self, :children, default_children_query)]
   end
 
   ##
@@ -106,34 +98,15 @@ module Type::AttributeGroups
   def default_attribute_groups
     values = work_package_attributes_by_default_group_key
 
-    ordered = []
+    default_groups.keys.each_with_object([]) do |groupkey, array|
-    default_groups.each_key do |groupkey|
       members = values[groupkey]
-      ordered << [groupkey, members.sort] if members.present?
+      array << [groupkey, members] if members.present?
     end
-
-    ordered
   end
 
-  ##
+  # TODO: remove once queries can be configured as well
-  # Collect active and inactive form configuration groups for editing.
+  def non_query_attribute_groups
-  def form_configuration_groups
+    attribute_groups.select { |g| g.is_a?(Type::AttributeGroup) }
-    available = work_package_attributes
-    # First we create a complete list of all attributes.
-    # Later we will remove those that are members of an attribute group.
-    # This way attributes that were created after the las group definitions
-    # will fall back into the inactives group.
-    inactive = available.clone
-
-    active_form = get_active_groups(available, inactive)
-    inactive_form = inactive
-                    .map { |key, attribute| attr_form_map(key, attribute) }
-                    .sort_by { |attr| attr[:translation] }
-
-    {
-      actives: active_form,
-      inactives: inactive_form
-    }
   end
 
   private
@@ -153,40 +126,25 @@ module Type::AttributeGroups
   end
 
   def default_group_key(key)
-    if custom_field?(key)
+    if CustomField.custom_field_attribute?(key)
       :other
     else
       default_group_map.fetch(key.to_sym, :details)
     end
   end
 
-  ##
-  # Collect active attributes from the current form configuration.
-  # Using the available attributes from +work_package_attributes+,
-  # determines which attributes are not used
-  def get_active_groups(available, inactive)
-    attribute_groups.map do |group|
-      extended_attributes =
-        group.second
-             .select { |key| inactive.delete(key) }
-             .map! { |key| attr_form_map(key, available[key]) }
-
-      [group[0], extended_attributes]
-    end
-  end
-
   def validate_attribute_group_names
     seen = Set.new
-    attribute_groups.each do |group_key, _|
+    attribute_groups.each do |group|
-      errors.add(:attribute_groups, :group_without_name) unless group_key.present?
+      errors.add(:attribute_groups, :group_without_name) unless group.key.present?
-      errors.add(:attribute_groups, :duplicate_group, group: group_key) if seen.add?(group_key).nil?
+      errors.add(:attribute_groups, :duplicate_group, group: group.key) if seen.add?(group.key).nil?
     end
   end
 
   def validate_attribute_groups
     valid_attributes = work_package_attributes.keys
-    attribute_groups.each do |_, attributes|
+    non_query_attribute_groups.each do |group|
-      attributes.each do |key|
+      group.attributes.each do |key|
         if key.is_a?(String) && valid_attributes.exclude?(key)
           errors.add(:attribute_groups, :attribute_unknown)
         end
@@ -197,10 +155,16 @@ module Type::AttributeGroups
   def work_package_attributes_by_default_group_key
     work_package_attributes
       .keys
-      .reject { |key| custom_field?(key) && !has_custom_field?(key) }
+      .reject { |key| CustomField.custom_field_attribute?(key) && !has_custom_field?(key) }
       .group_by { |key| default_group_key(key.to_sym) }
   end
 
+  def to_attribute_group_class(groups)
+    groups.map do |group|
+      Type::AttributeGroup.new(self, group[0], group[1])
+    end
+  end
+
   def default_children_query
     query = Query.new_default
     query.column_names = %w(id type subject)

app/models/type/attribute_groups_serializer.rb

@@ -32,25 +32,12 @@ module Type::AttributeGroupsSerializer
   def self.load(serialized_groups)
     return [] if serialized_groups.nil?
 
-    YAML.safe_load(serialized_groups, [Symbol]).each do |group|
+    YAML.safe_load(serialized_groups, [Symbol])
-      id = group[1][0].match /query_(\d+)/
-
-      if id
-        group[1][0] = Query.find(id)
-      end
-    end
   end
 
   def self.dump(groups)
-    serialized_groups = groups.each do |group|
+    non_query_groups = groups.reject { |g| g.is_a?(Type::QueryGroup) }
-      if group[1][0].is_a?(Query)
-        query = group[1][0]
-        query.save
-
-        group[1][0] = 'query_#{query.id}'
-      end
-    end
 
-    YAML.dump(serialized_groups)
+    YAML.dump(non_query_groups)
   end
 end

app/models/type/attributes.rb

@@ -149,19 +149,6 @@ module Type::Attributes
     end
   end
 
-  def attr_form_map(key, represented)
-    {
-      key: key,
-      is_cf: custom_field?(key),
-      is_required: represented[:required] && !represented[:has_default],
-      translation: self.class.translated_attribute_name(key, represented)
-    }
-  end
-
-  def custom_field?(attribute_name)
-    attribute_name.to_s.start_with? 'custom_field_'
-  end
-
   ##
   # Get all applicale work package attributes
   def work_package_attributes(merge_date: true)
@@ -178,7 +165,7 @@ module Type::Attributes
   # to the constraint validator.
   def passes_attribute_constraint?(attribute, project: nil)
     # Check custom field constraints
-    if custom_field?(attribute) && !project.nil?
+    if CustomField.custom_field_attribute?(attribute) && !project.nil?
       return custom_field_in_project?(attribute, project)
     end
 

app/models/type/form_group.rb

@@ -0,0 +1,60 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+class Type::FormGroup
+  attr_accessor :key,
+                :attributes,
+                :type
+
+  def initialize(type, key, attributes)
+    self.key = key
+    self.attributes = attributes
+    self.type = type
+  end
+
+  ##
+  # Translate the given attribute group if its internal
+  # (== if it's a symbol)
+  def translated_key
+    if key.is_a? Symbol
+      I18n.t(Type.default_groups[key])
+    else
+      key
+    end
+  end
+
+  def members
+    raise NotImplementedError
+  end
+
+  def active_members(project)
+    raise NotImplementedError
+  end
+end

app/models/type/query_group.rb

@@ -0,0 +1,41 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+class Type::QueryGroup < Type::FormGroup
+  alias :query :attributes
+
+  def members
+    [attributes]
+  end
+
+  def active_members(project)
+    [members]
+  end
+end

app/services/base_type_service.rb

@@ -71,11 +71,9 @@ class BaseTypeService
   def set_active_custom_fields
     active_cf_ids = []
 
-    type.attribute_groups.each do |_, members|
+    type.non_query_attribute_groups.each do |group|
-      next if members[0].is_a?(Query)
+      group.members.each do |attribute|
-
+        if CustomField.custom_field_attribute? attribute
-      members.each do |attribute|
-        if attribute.start_with? 'custom_field_'
           active_cf_ids << attribute.gsub(/^custom_field_/, '').to_i
         end
       end

app/views/types/form/_form_configuration.html.erb

@@ -27,9 +27,10 @@ See docs/COPYRIGHT.rdoc for more details.
 
 ++#%>
 
-<% form_attributes = @type.form_configuration_groups %>
+<% form_attributes = form_configuration_groups(@type) %>
+
 <section class="form--section">
-  <%= f.hidden_field :attribute_groups, value: @type.attribute_groups.map { |group| [group[0], group[1], (group[0].is_a? Symbol)] }.to_json %>
+  <%= f.hidden_field :attribute_groups, value: @type.non_query_attribute_groups.map { |group| [group.key, group.attributes, (group.key.is_a? Symbol)] }.to_json %>
   <div id="types-form-configuration" op-drag-scroll ng-controller="TypesFormConfigurationCtrl" ng-init="upsaleLink = <%= '"' + OpenProject::Static::Links.links[:upsale][:href] +  "/?utm_source=unknown&utm_medium=community-edition&utm_campaign=form-configuration" + '"' %>">
     <div class="grid-block wrap">
       <div class="grid-content small-12 large-10">
@@ -114,16 +115,16 @@ See docs/COPYRIGHT.rdoc for more details.
         </div>
         <div id="draggable-groups" dragula='"groups"'>
           <% form_attributes[:actives].each do |group, attributes| %>
-            <div class="type-form-conf-group" data-original-key="<%= group.to_s %>" data-key="<%= group %>" data-key-is-symbol="<%= group.is_a? Symbol %>">
+            <div class="type-form-conf-group" data-original-key="<%= group.key.to_s %>" data-key="<%= group.key %>" data-key-is-symbol="<%= group.key.is_a? Symbol %>">
               <div class="group-head">
                 <span class="group-handle icon-toggle"></span>
                 <group-edit-in-place
-                  name="<%= @type.translated_attribute_group(group) %>"
+                  name="<%= group.translated_key %>"
-                  key="<%= group %>"
+                  key="<%= group.key %>"
                   onvaluechange="groupNameChange"
                   onupsale="<%= EnterpriseToken.allows_to?(:edit_attribute_groups) ? '' : 'showEEOnlyHint' %>"
                   class="group-name">
-                  <%= @type.translated_attribute_group(group) %>
+                  <%= group.translated_key %>
                 </group-edit-in-place>
                 <span class="delete-group icon-small icon-close" ng-click="deleteGroup($event)"></span>
               </div>

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

@@ -69,56 +69,11 @@ module API
             false
           end
 
-          ##
-          # Return of a map of attribute => group name
-          def attribute_group_map(key)
-            return nil if type.nil?
-            @attribute_group_map ||= begin
-              attribute_groups.each_with_object({}) do |(group, attributes), hash|
-                attributes.each { |prop| hash[prop] = group }
-              end
-            end
-
-            @attribute_group_map[key]
-          end
-
-          def attribute_groups
-            return nil if type.nil?
-
-            @attribute_groups ||= begin
-              # It's important to deep_dup the attribute_groups
-              # as the operations would otherwise alter type's
-              # attribute_groups leading to unexpected side effects
-              type
-                .attribute_groups
-                .deep_dup
-                .map do |group|
-                group[1].map! do |prop|
-                  next unless prop.is_a?(Query) || type.passes_attribute_constraint?(prop, project: project)
-
-                  if prop.is_a?(Query)
-                    prop
-                  else
-                    convert_property(prop)
-                  end
-                end
-
-                group[1].compact!
-                group[0] = type.translated_attribute_group(group[0])
-                group
-              end
-            end
-          end
-
           private
 
           def contract
             raise NotImplementedError
           end
-
-          def convert_property(prop)
-            ::API::Utilities::PropertyNameConverter.from_ar_name(prop)
-          end
         end
       end
     end

lib/api/v3/work_packages/schema/form_configurations/attribute_representer.rb

@@ -32,6 +32,14 @@ module API
       module Schema
         module FormConfigurations
           class AttributeRepresenter < ::API::Decorators::Single
+            attr_accessor :project
+
+            def initialize(model, current_user:, project:, embed_links: false)
+              self.project = project
+
+              super(model, current_user: current_user, embed_links: embed_links)
+            end
+
             property :name,
                      exec_context: :decorator
 
@@ -43,11 +51,17 @@ module API
             end
 
             def name
-              represented[0]
+              represented.translated_key
             end
 
             def attributes
-              represented[1]
+              represented.active_members(project).map do |attribute|
+                convert_property(attribute)
+              end
+            end
+
+            def convert_property(attribute)
+              ::API::Utilities::PropertyNameConverter.from_ar_name(attribute)
             end
           end
         end

lib/api/v3/work_packages/schema/form_configurations/query_representer.rb

@@ -58,11 +58,11 @@ module API
             end
 
             def name
-              represented[0]
+              represented.translated_key
             end
 
             def query
-              represented[1][0]
+              represented.query
             end
           end
         end

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

@@ -53,7 +53,7 @@ module API
             def attribute_group(property)
               lambda do
                 key = property.to_s.gsub /^customField/, "custom_field_"
-                represented.attribute_group_map key
+                attribute_group_map key
               end
             end
 
@@ -253,14 +253,28 @@ module API
                                          has_default: true
 
           def attribute_groups
-            (represented.attribute_groups || []).map do |group|
+            (represented.type && represented.type.attribute_groups || []).map do |group|
-              klass = if group[1][0].is_a?(Query)
+              if group.is_a?(Type::QueryGroup)
-                        ::API::V3::WorkPackages::Schema::FormConfigurations::QueryRepresenter
+                ::API::V3::WorkPackages::Schema::FormConfigurations::QueryRepresenter
-                      else
+                  .new(group, current_user: current_user, embed_links: true)
-                        ::API::V3::WorkPackages::Schema::FormConfigurations::AttributeRepresenter
+              else
-                      end
+                ::API::V3::WorkPackages::Schema::FormConfigurations::AttributeRepresenter
-              klass.new(group, current_user: current_user, embed_links: true)
+                  .new(group, current_user: current_user, project: represented.project, embed_links: true)
+              end
+            end
+          end
+
+          ##
+          # Return a map of attribute => group name
+          def attribute_group_map(key)
+            return nil if represented.type.nil?
+            @attribute_group_map ||= begin
+              represented.type.attribute_groups.each_with_object({}) do |group, hash|
+                Array(group.active_members(represented.project)).each { |prop| hash[prop] = group.translated_key }
+              end
             end
+
+            @attribute_group_map[key]
           end
         end
       end

spec/helpers/types_helper_spec.rb

@@ -0,0 +1,81 @@
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+require 'spec_helper'
+
+describe TypesHelper, type: :helper do
+  let(:type) { FactoryGirl.build_stubbed(:type) }
+
+  describe "#form_configuration_groups" do
+    it "returns a Hash with the keys :actives and :inactives Arrays" do
+      expect(helper.form_configuration_groups(type)[:actives]).to be_an Array
+      expect(helper.form_configuration_groups(type)[:inactives]).to be_an Array
+    end
+
+    describe ":inactives" do
+      subject { helper.form_configuration_groups(type)[:inactives] }
+
+      before do
+        allow(type)
+          .to receive(:attribute_groups)
+          .and_return [::Type::AttributeGroup.new(type, 'group one', ["assignee"])]
+      end
+
+      it 'contains Hashes ordered by key :translation' do
+        # The first left over attribute should currently be "date"
+        expect(subject.first[:translation]).to be_present
+        expect(subject.first[:translation] <= subject.second[:translation]).to be_truthy
+      end
+
+      # The "assignee" is in "group one". It should not appear in :inactives.
+      it 'does not contain attributes that do not exist anymore' do
+        expect(subject.map { |inactive| inactive[:key] }).to_not include "assignee"
+      end
+    end
+
+    describe ":actives" do
+      subject { helper.form_configuration_groups(type)[:actives] }
+
+      before do
+        allow(type)
+          .to receive(:attribute_groups)
+          .and_return [::Type::AttributeGroup.new(type, 'group one', ["date"])]
+      end
+
+      it 'has a proper structure' do
+        # The group's name/key
+        expect(subject.first.first.key).to eq "group one"
+
+        # The groups attributes
+        expect(subject.first.second).to be_an Array
+        expect(subject.first.second.first[:key]).to eq "date"
+        expect(subject.first.second.first[:translation]).to eq "Date"
+      end
+    end
+  end
+end

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

@@ -103,19 +103,4 @@ describe ::API::V3::WorkPackages::Schema::TypedWorkPackageSchema do
       expect(subject.assignable_custom_field_values(version_cf)).to be_nil
     end
   end
-
-  describe '#attribute_groups' do
-    it "has no side effects on type's #attribute_groups" do
-      before = [["People", ["assignee", "responsible"]],
-                ["Estimates and time", ["estimated_time", "spent_time"]],
-                ["Details", ["category", "date", "priority", "version"]],
-                ["Other", ["percentage_done"]]]
-
-      type.attribute_groups = before
-
-      subject.attribute_groups
-
-      expect(type.attribute_groups).to eql before
-    end
-  end
 end

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

@@ -32,21 +32,21 @@ describe ::API::V3::WorkPackages::Schema::WorkPackageSchemaRepresenter do
   include API::V3::Utilities::PathHelper
 
   let(:project) { FactoryGirl.build_stubbed(:project_with_types) }
-  let(:type) { FactoryGirl.build_stubbed(:type) }
+  let(:wp_type) { project.types.first }
   let(:custom_field) { FactoryGirl.build_stubbed(:custom_field) }
-  let(:work_package) { FactoryGirl.build_stubbed(:stubbed_work_package, project: project, type: project.types.first) }
+  let(:work_package) { FactoryGirl.build_stubbed(:stubbed_work_package, project: project, type: wp_type) }
   let(:current_user) do
     FactoryGirl.build_stubbed(:user)
   end
   let(:attribute_query) { FactoryGirl.build_stubbed(:query) }
   let(:attribute_groups) do
-    [["People", %w(assignee responsible)],
+    [Type::AttributeGroup.new(wp_type, "People", %w(assignee responsible)),
-     ["Estimates and time", %w(estimatedTime spentTime)],
+     Type::AttributeGroup.new(wp_type, "Estimates and time", %w(estimatedTime spentTime)),
-     ["Children", [attribute_query]]]
+     Type::QueryGroup.new(wp_type, "Children", attribute_query)]
   end
   let(:schema) do
     ::API::V3::WorkPackages::Schema::SpecificWorkPackageSchema.new(work_package: work_package).tap do |schema|
-      allow(schema)
+      allow(wp_type)
         .to receive(:attribute_groups)
         .and_return(attribute_groups)
       allow(schema)

spec/models/type/attribute_groups_spec.rb

@@ -39,20 +39,71 @@ describe ::Type, type: :model do
   end
 
   describe "#attribute_groups" do
-    it 'returns #default_attribute_groups if not yet set' do
+    shared_examples_for 'appends the children query' do |position|
-      expect(type.read_attribute(:attribute_groups)).to be_empty
+      it "at position #{position}" do
-      expect(type.attribute_groups).to_not be_empty
+        group = type.attribute_groups[position]
-      expect(type.attribute_groups).to eq type.default_attribute_groups
+
+        expect(group.key).to eql :children
+        query = group.members[0]
+
+        expect(query.class).to eql Query
+
+        expect(query.filters.length).to eql(1)
+
+        filter = query.filters[0]
+
+        expect(filter.name).to eql(:parent)
+
+        expect(query.column_names).to eql(%i(id type subject))
+        expect(query.show_hierarchies).to be_falsey
+      end
+    end
+
+    shared_examples_for 'returns default attributes' do
+      it do
+        expect(type.read_attribute(:attribute_groups)).to be_empty
+
+        attribute_groups = type.attribute_groups[0..2].map do |group|
+          [group.key, group.attributes]
+        end
+        expect(attribute_groups).to eql type.default_attribute_groups
+      end
+
+      it_behaves_like 'appends the children query', 3
+    end
+
+    context 'with attributes provided' do
+      before do
+        type.attribute_groups = [['foo', []], ['bar', %w(blubs date)]]
+      end
+
+      it 'removes unknown attributes from a group' do
+        group = type.attribute_groups[1]
+
+        expect(group.key).to eql 'bar'
+        expect(group.members).to eql ['date']
+      end
+
+      it 'keeps groups without attributes' do
+        group = type.attribute_groups[0]
+
+        expect(group.key).to eql 'foo'
+        expect(group.members).to eql []
+      end
+
+      it_behaves_like 'appends the children query', 2
     end
 
-    it 'removes unknown attributes from a group' do
+    context 'with empty attributes provided' do
-      type.attribute_groups = [['foo', ['bar', 'date']]]
+      before do
-      expect(type.attribute_groups).to eq [['foo', ['date']]]
+        type.attribute_groups = []
+      end
+
+      it_behaves_like 'returns default attributes'
     end
 
-    it 'keeps groups without attributes' do
+    context 'with no attributes provided' do
-      type.attribute_groups = [['foo', []], ['bar', ['date']]]
+      it_behaves_like 'returns default attributes'
-      expect(type.attribute_groups).to eq [['foo', []], ['bar', ['date']]]
     end
   end
 
@@ -83,14 +134,6 @@ describe ::Type, type: :model do
         group_members.nil? || group_members.size.zero?
       end).to be_falsey
     end
-
-    it 'returns the default children query' do
-      children_group = subject.detect { |group| group[0] == :children }
-
-      expect(children_group).to_not be_nil
-      expect(children_group[1].length).to eql 1
-      expect(children_group[1][0]).to be_a(Query)
-    end
   end
 
   describe "#validate_attribute_groups" do
@@ -123,52 +166,7 @@ describe ::Type, type: :model do
     it 'passes validation for reset' do
       # A reset is to save an empty Array
       type.attribute_groups = []
-      expect(type.save).to be_truthy
+      expect(type).to be_valid
-      expect(type.attribute_groups).to eq type.default_attribute_groups
-    end
-  end
-
-  describe "#form_configuration_groups" do
-    it "returns a Hash with the keys :actives and :inactives Arrays" do
-      expect(type.form_configuration_groups[:actives]).to be_an Array
-      expect(type.form_configuration_groups[:inactives]).to be_an Array
-    end
-
-    describe ":inactives" do
-      subject { type.form_configuration_groups[:inactives] }
-
-      before do
-        type.attribute_groups = [["group one", ["assignee"]]]
-      end
-
-      it 'contains Hashes ordered by key :translation' do
-        # The first left over attribute should currently be "date"
-        expect(subject.first[:translation]).to be_present
-        expect(subject.first[:translation] <= subject.second[:translation]).to be_truthy
-      end
-
-      # The "assignee" is in "group one". It should not appear in :inactives.
-      it 'does not contain attributes that do not exist anymore' do
-        expect(subject.map { |inactive| inactive[:key] }).to_not include "assignee"
-      end
-    end
-
-    describe ":actives" do
-      subject { type.form_configuration_groups[:actives] }
-
-      before do
-        allow(type).to receive(:attribute_groups).and_return [["group one", ["date"]]]
-      end
-
-      it 'has a proper structure' do
-        # The group's name/key
-        expect(subject.first.first).to eq "group one"
-
-        # The groups attributes
-        expect(subject.first.second).to be_an Array
-        expect(subject.first.second.first[:key]).to eq "date"
-        expect(subject.first.second.first[:translation]).to eq "Date"
-      end
     end
   end
 
@@ -187,10 +185,6 @@ describe ::Type, type: :model do
       # Enforce fresh lookup of groups
       OpenProject::Cache.clear
 
-      # Is in inactive group
-      form = type.form_configuration_groups
-      expect(form[:inactives][0][:key]).to eq(cf_identifier.to_s)
-
       # Can be enabled
       type.attribute_groups = [['foo', [cf_identifier.to_s]]]
       expect(type.save).to be_truthy