Fix adding and moving fields between fieldsets.

news/86.bugfix

@@ -0,0 +1,2 @@
+Fix adding and moving fields between fieldsets
+[frapell]

plone/schemaeditor/browser/schema/add_field.py

@@ -11,8 +11,8 @@
 from Products.statusmessages.interfaces import IStatusMessage
 from z3c.form import field
 from z3c.form import form
-from z3c.form.browser.text import TextWidget
 from z3c.form.interfaces import HIDDEN_MODE
+from z3c.form.interfaces import NO_VALUE
 from z3c.form.interfaces import WidgetActionExecutionError
 from zope.cachedescriptors.property import Lazy as lazy_property
 from zope.component import getAdapters
@@ -45,6 +45,10 @@ def create(self, data):
         extra = {}
         factory = data.pop('factory')
 
+        # remove fieldset_id from data
+        if 'fieldset_id' in data:
+            fieldset_id = data.pop('fieldset_id')
+
         # split regular attributes and extra ones
         for key in list(data.keys()):
             if key not in self._schema:
@@ -67,7 +71,14 @@ def create(self, data):
     def add(self, new_field):
         schema = self.context.schema
         fieldset_id = int(self.request.form.get('fieldset_id', 0))
-        position = new_field_position(schema, fieldset_id)
+
+        if self.widgets:
+            fieldset_widget = self.widgets.get('fieldset_id')
+            if fieldset_widget:
+                fieldset_id = int(fieldset_widget.extract())
+            position = new_field_position(schema, fieldset_id, new_field=True)
+        else:
+            position = new_field_position(schema, fieldset_id)
 
         editable_schema = IEditableSchema(schema)
         try:
@@ -91,17 +102,12 @@ def add(self, new_field):
 
     def updateWidgets(self):
         super(FieldAddForm, self).updateWidgets()
-        fieldset_id = int(self.request.form.get('fieldset_id', 0))
-        if fieldset_id:
-            # add fieldset_id from GET parameter as hidden field, so that
-            # ``add`` method at the end of the form lifecycle can read it.
-            fieldset_id_widget = TextWidget(self.request)
-            fieldset_id_widget.name = 'fieldset_id'
-            fieldset_id_widget.value = fieldset_id
+        fieldset_id_widget = self.widgets.get('fieldset_id')
+        if fieldset_id_widget:
+            if not fieldset_id_widget.value or fieldset_id_widget.value == NO_VALUE:
+                fieldset_id = int(self.request.form.get('fieldset_id', 0))
+                fieldset_id_widget.value = fieldset_id
             fieldset_id_widget.mode = HIDDEN_MODE
-            # Uhm. z3c.form widgets doesn't have an API for extending a
-            # schema-generated form. Using internal ``_data_values``...
-            self.widgets._data_values.append(fieldset_id_widget)
 
     def nextURL(self):
         return '@@add-field'

plone/schemaeditor/browser/schema/add_fieldset.py

@@ -4,6 +4,7 @@
 from plone.schemaeditor.utils import SchemaModifiedEvent
 from plone.supermodel.interfaces import FIELDSETS_KEY
 from plone.supermodel.model import Fieldset
+from plone.supermodel.utils import mergedTaggedValueList
 from plone.z3cform.layout import wrap_form
 from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile
 from Products.statusmessages.interfaces import IStatusMessage
@@ -27,8 +28,13 @@ def create(self, data):
     def add(self, new_fieldset):
         schema = self.context.schema
         fieldsets = schema.queryTaggedValue(FIELDSETS_KEY, [])
+        # Also check on fieldsets coming from behaviors
+        extra_fieldsets = list()
+        for additional_schema in self.context.additionalSchemata:
+            for elem in mergedTaggedValueList(additional_schema, FIELDSETS_KEY):
+                extra_fieldsets.append(elem)
 
-        for fieldset in fieldsets:
+        for fieldset in (fieldsets + extra_fieldsets):
             if fieldset.__name__ == new_fieldset.__name__:
                 msg = _(
                     u'Please select a fieldset name that is not already used.'

plone/schemaeditor/browser/schema/listing.py

@@ -4,6 +4,7 @@
 from plone.schemaeditor import _
 from plone.schemaeditor.interfaces import IFieldFactory
 from plone.schemaeditor.utils import SchemaModifiedEvent
+from plone.supermodel.interfaces import FIELDSETS_KEY
 from plone.z3cform.layout import FormWrapper
 from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile
 from z3c.form import button
@@ -93,8 +94,17 @@ def edit_url(self, field):
                 field.__name__,
             )
 
-    def delete_url(self, field):
+    def can_delete_fieldset(self, fieldset):
+        can_delete = False
+        if fieldset != self:
+            added_fieldsets = self.context.schema.queryTaggedValue(FIELDSETS_KEY, [])
+            for custom_fieldset in added_fieldsets:
+                if custom_fieldset.__name__ == fieldset.__name__:
+                    can_delete = True
+                    break
+        return can_delete
 
+    def delete_url(self, field):
         if field.__name__ in self.context.fieldsWhichCannotBeDeleted:
             return
         url = '{0}/{1}/@@delete'.format(

plone/schemaeditor/browser/schema/schema_listing.pt

@@ -74,14 +74,16 @@
     <metal:fields-slot metal:fill-slot="fields">
 
       <tal:block tal:repeat="group python:[view] + list(view.groups)">
-        <fieldset tal:define="fieldset_name repeat/group/index"
+        <fieldset tal:define="fieldset_name repeat/group/index;
+                              can_delete python:view.can_delete_fieldset(group);"
                   tal:attributes="id string:fieldset-${fieldset_name};
-                                  class string:kssattr-fieldset-${fieldset_name}">
+                                  class string:kssattr-fieldset-${fieldset_name};
+                                  data-can-add-fields python:(repeat['group'].start or can_delete) and 'true' or 'false'">
 
           <legend tal:define="group_name python:group.label or view.default_fieldset_label"
                   tal:content="group_name">Fieldset name</legend>
 
-          <a id="delete-fieldset-${fieldset_name}" tal:condition="python:context.enableFieldsets and group.__name__ != view.__name__"
+          <a id="delete-fieldset-${fieldset_name}" class="btn btn-sm btn-danger" tal:condition="python:context.enableFieldsets and can_delete"
              href="${context/absolute_url}/@@delete-fieldset?name=${python:group.__name__}&_authenticator=${context/@@authenticator/token}"
                   i18n:translate="delete_fieldset_hellip">Delete fieldset
           </a>

plone/schemaeditor/interfaces.py

@@ -12,6 +12,7 @@
 from zope.schema import ASCIILine
 from zope.schema import Bool
 from zope.schema import Choice
+from zope.schema import Int
 from zope.schema import Object
 from zope.schema import Text
 from zope.schema import TextLine
@@ -157,6 +158,12 @@ def isValidFieldName(value):
 
 class INewField(Interface):
 
+    fieldset_id = Int(
+        title=_(u'Fieldset ID'),
+        description=_(u'Used to decide where to put this field.'),
+        required=True,
+    )
+
     title = TextLine(
         title=_(u'Title'),
         required=True

plone/schemaeditor/tests/robot/test_fields.robot

@@ -10,6 +10,7 @@ Test Setup  Run Keywords  Plone test setup
 Test Teardown  Run keywords  Plone test teardown
 
 *** Variables ***
+${SELENIUM_TIMEOUT}  20
 
 *** Test Cases ***
 
@@ -86,23 +87,29 @@ Add a fieldSet and move a field into this fieldset
     Go to dexterity types configuration
     Add content type  Contact info  contact_info
     Add field  Address  address  Text
-    Click Link  Add new fieldset…
-    Input text for sure  form-widgets-label  Personal information
-    Set Focus to Element  form-widgets-__name__
-    Wait until keyword succeeds  10  1  Textfield Value Should Be  form-widgets-__name__  personal_information
-    Wait For Then Click Element  css=.modal-footer #form-buttons-add
-    Wait overlay is closed
-    Wait until page contains  Personal information
+    Add fieldset  Personal information  personal_information
 
 #    Mouse Down  xpath=//*[@data-field_id="address"][1]
 #    Mouse Over  xpath=//form//nav[@class="autotoc-nav"]/a[@data-fieldset_drag_id="1"]
 #    Mouse Up  xpath=//*[@data-fieldset_drag_id="1"][1]
 #    Wait Until Keyword Succeeds  10  1  Element should not be visible  css=.fieldPreview[data-field_id="address"]
-#
 #    Click Element  xpath=//form//nav[@class="autotoc-nav"]/a[@data-fieldset_drag_id="1"]
 #    Wait Until Keyword Succeeds  10  1  Element should be visible  css=.fieldPreview[data-field_id="address"]
 
 
+Add a fieldSet and add a field into this fieldset
+
+    Go to dexterity types configuration
+    Add content type  Contact info  contact_info
+    Add fieldset  Personal information  personal_information
+    Click Element  xpath=//form//nav[@class="autotoc-nav"]/a[@data-fieldset_drag_id="1"]
+    Add field  Address  address  Text
+    Click Element  xpath=//form//nav[@class="autotoc-nav"]/a[@data-fieldset_drag_id="0"]
+    Wait Until Keyword Succeeds  10  1  Element should not be visible  css=.fieldPreview[data-field_id="address"]
+    Click Element  xpath=//form//nav[@class="autotoc-nav"]/a[@data-fieldset_drag_id="1"]
+    Wait Until Keyword Succeeds  10  1  Element should be visible  css=.fieldPreview[data-field_id="address"]
+
+
 #Add a fieldSet and add a field into this fieldset
 #
 #    Set Window Size  1200  1200
@@ -184,6 +191,18 @@ Add field
     Wait For Then Click Element  css=.modal-footer #form-buttons-add
     Wait overlay is closed
 
+Add fieldset
+    [Arguments]    ${fieldset_title}    ${fieldset_id}
+    [Documentation]    Add fieldset in current dexterity content type
+
+    Click Link  Add new fieldset…
+    Input text for sure  form-widgets-label  ${fieldset_title}
+    Set Focus to Element  form-widgets-__name__
+    Wait until keyword succeeds  10  1  Textfield Value Should Be  form-widgets-__name__  ${fieldset_id}
+    Wait For Then Click Element  css=.modal-footer #form-buttons-add
+    Wait overlay is closed
+    Wait until page contains  ${fieldset_title}
+
 Open field settings
     [Arguments]    ${field_id}
     ${locator}  Set Variable  //div[@data-field_id='${field_id}']//a[contains(@class, 'fieldSettings pat-plone-modal')]

plone/schemaeditor/utils.py

@@ -56,7 +56,7 @@ def get_fieldset_from_index(schema, index):
     return fieldsets[index] if index >= 0 else None
 
 
-def new_field_position(schema, fieldset_id=None):
+def new_field_position(schema, fieldset_id=None, new_field=False):
     """Get the position for a new field in a schema's fieldset.
     If fieldset_id is ``None`` or ``0``, the default fieldset is used.
     """
@@ -70,12 +70,15 @@ def new_field_position(schema, fieldset_id=None):
     else:
         # First we get the first of the fieldsets after the new one
         fieldsets = schema.queryTaggedValue(FIELDSETS_KEY, [])
-        for fs in fieldsets[fieldset_id + 1:]:
+        for fs in fieldsets[fieldset_id:]:
             if len(fs.fields) > 0:
-                position = ordered_field_ids.index(fs.fields[0]) - 1
+                position = ordered_field_ids.index(fs.fields[0])
                 break
         else:
             position = len(ordered_field_ids) - 1
+            if new_field:
+                # Special case when adding a new field
+                position += 1
 
     return position