Reindex the collections in CollectionType to prevent issues related of changing the order between the requests.

src/Entity/Parts/Part.php

@@ -96,7 +96,6 @@ class Part extends AttachmentContainingDBElement
      * @Assert\Valid()
      * @ORM\OneToMany(targetEntity="App\Entity\Parameters\PartParameter", mappedBy="element", cascade={"persist", "remove"}, orphanRemoval=true)
      * @ORM\OrderBy({"group" = "ASC" ,"name" = "ASC"})
-     * @ORM\OrderBy({"group" = "ASC" ,"name" = "ASC"})
      */
     protected $parameters;
 

src/Form/AdminPages/BaseEntityAdminForm.php

@@ -132,6 +132,7 @@ class BaseEntityAdminForm extends AbstractType
             'allow_add' => true,
             'allow_delete' => true,
             'label' => false,
+            'reindex_enable' => true,
             'disabled' => ! $this->security->isGranted($is_new ? 'create' : 'edit', $entity),
             'entry_options' => [
                 'data_class' => $options['attachment_class'],
@@ -158,6 +159,7 @@ class BaseEntityAdminForm extends AbstractType
             'allow_add' => $this->security->isGranted($is_new ? 'create' : 'edit', $entity),
             'allow_delete' => $this->security->isGranted($is_new ? 'create' : 'edit', $entity),
             'disabled' => ! $this->security->isGranted($is_new ? 'create' : 'edit', $entity),
+            'reindex_enable' => true,
             'label' => false,
             'by_reference' => false,
             'prototype_data' => new $options['parameter_class'](),

src/Form/CollectionTypeExtension.php

@@ -0,0 +1,120 @@
+<?php
+/**
+ * This file is part of Part-DB (https://github.com/Part-DB/Part-DB-symfony).
+ *
+ * Copyright (C) 2019 - 2020 Jan Böhmer (https://github.com/jbtronics)
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published
+ * by the Free Software Foundation, either version 3 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 Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+namespace App\Form;
+
+
+use Doctrine\Common\Collections\ArrayCollection;
+use Doctrine\Common\Collections\Collection;
+use Symfony\Component\Form\AbstractTypeExtension;
+use Symfony\Component\Form\Extension\Core\Type\CollectionType;
+use Symfony\Component\Form\FormBuilder;
+use Symfony\Component\Form\FormBuilderInterface;
+use Symfony\Component\Form\FormConfigBuilder;
+use Symfony\Component\Form\FormEvent;
+use Symfony\Component\Form\FormEvents;
+use Symfony\Component\OptionsResolver\Options;
+use Symfony\Component\OptionsResolver\OptionsResolver;
+use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
+
+/**
+ * Perform a reindexing on CollectionType elements, by assigning the database id as index.
+ * This prevents issues when the collection that is edited uses a OrderBy annotation and therefore the direction of the
+ * elements can change during requests.
+ * Must me enabled by setting reindex_enable to true in Type options.
+ * @package App\Form
+ */
+class CollectionTypeExtension extends AbstractTypeExtension
+{
+    protected $propertyAccess;
+
+    public function __construct(PropertyAccessorInterface $propertyAccess)
+    {
+        $this->propertyAccess = $propertyAccess;
+    }
+
+    public static function getExtendedTypes(): iterable
+    {
+        return [CollectionType::class];
+    }
+
+    public function configureOptions(OptionsResolver $resolver)
+    {
+        /*$resolver->setDefault('error_mapping', function (Options $options) {
+                                    $options->
+                                });*/
+
+        $resolver->setDefaults([
+                                   'reindex_enable' => false,
+                                   'reindex_prefix' => 'db_',
+                                   'reindex_path' => 'id',
+                               ]);
+
+        $resolver->setAllowedTypes('reindex_enable', 'bool');
+        $resolver->setAllowedTypes('reindex_prefix', 'string');
+        $resolver->setAllowedTypes('reindex_path', 'string');
+    }
+
+    public function buildForm(FormBuilderInterface $builder, array $options)
+    {
+        $builder->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) use ($options) {
+            $data = $event->getData();
+            $config = $event->getForm()->getConfig();
+            //If enabled do a reindexing of the collection
+            if ($options['reindex_enable'] && $data instanceof Collection) {
+                $reindexed_data = new ArrayCollection();
+
+                $error_mapping = [];
+
+                foreach ($data->toArray() as $key => $item) {
+                    $index = $options['reindex_prefix'] . $this->propertyAccess->getValue($item, $options['reindex_path']);
+                    $error_mapping['[' . $key . ']'] = $index;
+                    $reindexed_data->set($index, $item);
+                }
+                $event->setData($reindexed_data);
+
+                //Add error mapping, so that validator error are mapped correctly to the new index fields
+                if ($config instanceof FormBuilder && empty($config->getOption('error_mapping'))) {
+                    $this->setOption($config, 'error_mapping', $error_mapping);
+                }
+            }
+        }, 100); //We need to have a higher priority then the PRE_SET_DATA listener on CollectionType
+    }
+
+    /**
+     * Set the option of the form.
+     * This a bit hacky cause we access private properties....
+     * @param $builder
+     * @param  string  $option
+     * @param $value
+     * @throws \ReflectionException
+     */
+    public function setOption(FormBuilder $builder, string $option, $value): void
+    {
+        //We have to use FormConfigBuilder::class here, because options is private and not available in sub classes
+        $reflection = new \ReflectionClass(FormConfigBuilder::class);
+        $property = $reflection->getProperty('options');
+        $property->setAccessible(true);
+        $tmp = $property->getValue($builder);
+        $tmp[$option] = $value;
+        $property->setValue($builder, $tmp);
+        $property->setAccessible(false);
+    }
+}

src/Form/Part/OrderdetailType.php

@@ -114,6 +114,7 @@ class OrderdetailType extends AbstractType
                 'allow_add' => $this->security->isGranted('@parts_prices.create'),
                 'allow_delete' => $this->security->isGranted('@parts_prices.delete'),
                 'label' => false,
+                'reindex_enable' => true,
                 'prototype_data' => $dummy_pricedetail,
                 'by_reference' => false,
                 'entry_options' => [

src/Form/Part/PartBaseType.php

@@ -224,6 +224,7 @@ class PartBaseType extends AbstractType
             'entry_type' => PartLotType::class,
             'allow_add' => $this->security->isGranted('lots.create', $part),
             'allow_delete' => $this->security->isGranted('lots.delete', $part),
+            'reindex_enable' => true,
             'label' => false,
             'entry_options' => [
                 'measurement_unit' => $part->getPartUnit(),
@@ -237,6 +238,7 @@ class PartBaseType extends AbstractType
             'entry_type' => AttachmentFormType::class,
             'allow_add' => $this->security->isGranted('attachments.create', $part),
             'allow_delete' => $this->security->isGranted('attachments.delete', $part),
+            'reindex_enable' => true,
             'label' => false,
             'entry_options' => [
                 'data_class' => PartAttachment::class,
@@ -257,6 +259,7 @@ class PartBaseType extends AbstractType
             'entry_type' => OrderdetailType::class,
             'allow_add' => $this->security->isGranted('orderdetails.create', $part),
             'allow_delete' => $this->security->isGranted('orderdetails.delete', $part),
+            'reindex_enable' => true,
             'label' => false,
             'by_reference' => false,
             'prototype_data' => new Orderdetail(),
@@ -271,6 +274,7 @@ class PartBaseType extends AbstractType
             'allow_add' => $this->security->isGranted('parameters.create', $part),
             'allow_delete' => $this->security->isGranted('parameters.delete', $part),
             'label' => false,
+            'reindex_enable' => true,
             'by_reference' => false,
             'prototype_data' => new PartParameter(),
             'entry_options' => [

src/Form/UserAdminForm.php

@@ -244,6 +244,7 @@ class UserAdminForm extends AbstractType
             'allow_add' => true,
             'allow_delete' => true,
             'label' => false,
+            'reindex_enable' => true,
             'entry_options' => [
                 'data_class' => $options['attachment_class'],
             ],