Convert the "Field storage edit" form to an actual entity form

Awesome! +1 rtbc
just a nit

+++ b/core/modules/field_ui/src/Form/FieldStorageEditForm.php
@@ -7,116 +7,87 @@
+  protected $entity;

is this for type hinting? entity form already have this one defined

Those two patches are definitely on my radar, didn't have a chance to get to them so far.

@amateescu+++++++

No time to look into the actual FieldStorageConfigEditForm tonight, just looked at the surroundings ;-)

1. +++ b/core/modules/comment/src/Tests/CommentFieldsTest.php
   @@ -129,7 +129,7 @@ public function testCommentFieldCreate() {
   -      'field_storage[settings][comment_type]' => 'user_comment_type',
   +      'settings[comment_type]' => 'user_comment_type',
   

   <3

2. +++ b/core/modules/field_ui/src/Routing/RouteSubscriber.php
   @@ -79,8 +79,8 @@ protected function alterRoutes(RouteCollection $collection) {
   -          array('_form' => '\Drupal\field_ui\Form\FieldStorageEditForm') + $defaults,
   -          array('_entity_access' => 'field_config.update'),
   +          array('_entity_form' => 'field_storage_config.edit') + $defaults,
   +          array('_permission' => 'administer ' . $entity_type_id . ' fields'),
   

   Wondering about this - should entity forms use entity access or dedicated permissions directly ?

   Even though the current code is definitely weird :
   - FieldStorageConfig currently defines no access handler
   - the existing routing code "borrows" *field_config*.update for this route about *field_storage_config*
   - field_config does define an access handler, FieldConfigAccessControlHandler, which grants access based on the "administer [entity_type] fields'

   I'm +++ on bringing some sanity here, just wondering whether we should stay with the entity access on the route and add a proper FieldStorageConfigAccessControlHandler ?

I think it's ready just two minor nits.

1. +++ b/core/modules/field_ui/src/Form/FieldStorageConfigEditForm.php
   @@ -2,121 +2,92 @@
   +    $ids = (object) array('entity_type' => $form_state->get('entity_type_id'), 'bundle' => $form_state->get('bundle'), 'entity_id' => NULL);
   

2. Can we change it to multi line now?

   +++ b/core/modules/field_ui/src/Form/FieldStorageConfigEditForm.php
   @@ -2,121 +2,92 @@
   +  public function buildForm(array $form, FormStateInterface $form_state, $field_config = NULL) {
   

   $field_config typehinting missing and function doc also needs update.

I manually tested the patch everything was working fine. So RTBC

I didn't know delete field page was showing 'Configuration updates' details so this is same in the HEAD and with this patch of course we are not doing anything with delete form in this issue.

No additional remarks for FieldStorageConfigEditForm, except :

+++ b/core/modules/field_ui/src/Form/FieldStorageConfigEditForm.php
@@ -144,79 +122,70 @@ public function buildForm(array $form, FormStateInterface $form_state, FieldConf
-    // Build the non-configurable field values.
-    $form['field_storage']['field_name'] = array('#type' => 'value', '#value' => $field_storage->getName());
-    $form['field_storage']['type'] = array('#type' => 'value', '#value' => $field_storage->getType());
-    $form['field_storage']['module'] = array('#type' => 'value', '#value' => $field_storage->getTypeProvider());
-    $form['field_storage']['translatable'] = array('#type' => 'value', '#value' => $field_storage->isTranslatable());

oh, yeah, these have been useless since probably CCK D6 :-)

RTBC +1

Hah ! I *knew* you'd say that :-p

UX improvements to field UI - yep we want that. Committed 030f536 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.