Field listings operations cannot be altered

trying to inject module handler...

Seems EntityManager is used in submit.

Updating tag.

The thing is.. we do want to turn that screen into an EntityListController, and that should happen in #1963340: Change field UI so that adding a field is a separate task.

Sorry for eating the tag.

Updating sprint tag - LONDON_2013_JULY

Getting this in separately, and then adjust the code if/when the "manage fields" is turned to a ListController, sounds fine by me ?

+++ b/core/modules/field_ui/lib/Drupal/field_ui/FieldOverview.phpundefined
@@ -10,12 +10,44 @@
+  public function __construct(EntityManager $entity_manager, ModuleHandlerInterface $module_handler) {
+    $this->entityManager = $entity_manager;
+    $this->moduleHandler = $module_handler;
+  }

Why are we also adding an entity manager ? Doesn't look like it's used anywhere (nor documented) ?

I looked this over. I thought we should have it ready, in case we decide to go ahead with it.
Only thing I could find was missing descriptions for @params
and I put the new use statements as close to alphabetical as I could, only changing the lines already changed.

Everything else looks fine. I think I might be starting to understand all the things to check with injection in terms of the __contstruct, create, having the parameters match and the order of things set in the static.

the entitymanager property comes from OverviewBase

--

I didn't try this with config_translation yet.

@yched

I asked myself the same question about the entitymanager.

we are not using it in the new code in this patch, but it was already used elsewhere.
For example, in the submitForm()

      // Create the field and instance.
      try {
        $this->entityManager->getStorageController('field_entity')->create($field)->save();
        $new_instance = $this->entityManager->getStorageController('field_instance')->create($instance);
        $new_instance->save();

And that was working before this patch added it, because it was inheriting the property and the __construct and create from OverviewBase since
class FieldOverview extends OverviewBase

(At least that is what I think... let me know if otherwise.)

tested it with config_translation without and with the patch. :)

Ah, right, entityManager is defined and documented in the parent class.

No biggie, but I think then it would be slightly more correct if FieldOverview::__construct() did:

  public function __construct(EntityManager $entity_manager, ModuleHandlerInterface $module_handler) {
    parent::__construct(EntityManager $entity_manager);
    $this->moduleHandler = $module_handler;
  }

Other than that, looks good to me :-)

@yched That makes sense, then if the construct there changes, we get those changes here automatically.
My syntax was a bit different, but did #16.

Hopefully this will be ready now.

This looks good if it comes back green.

green. cool.

rerolled because @YesCT suggested to do it :-)

There was a Conflict in core/modules/field_ui/lib/Drupal/field_ui/FieldOverview.php.
So i added
use Drupal\Core\Entity\Field\FieldTypePluginManager;

another try, couldn't find a php synax error, but fixed the formatting a bit.

in #22 too much formatting was done.

in #22 too much formatting was done.

uploaded wrong patch in #23

needs review

The patch tries to add a second __construct() method while there is already one :-)

@bechtold is here at the camp, so I will see if they can do it tonight.

#2047993-23: Remove current uses of field_info_*_types() / field_info_*_settings() added the same constructor and create method as our rerolled patch #26.

We fixed it.

FAILED: [[SimpleTest]]: [MySQL] 57,920 pass(es), 2 fail(s), and 0 exception(s).

HTTP response expected 204, actual 400 Browser NodeTest.php 70 Drupal\rest\Tests\NodeTest->testNodes()
Value '0\'v;Vlbc' is equal to value '|~-iJ\\1`'. Other NodeTest.php 74 Drupal\rest\Tests\NodeTest->testNodes()

I think that is a random failure. (#2056713: Random test failure in Drupal\rest\Tests\NodeTest)

#31: 2045043-filter-alter-31.patch queued for re-testing.

#31: 2045043-filter-alter-31.patch queued for re-testing.

Right, I forgot that #2047993: Remove current uses of field_info_*_types() / field_info_*_settings() would conflict. Thanks for the detective work!

while working on this testing I noticed a different problem.
#2057227: Field instance needs uri() method different from the default

The new hook needs an example in field.api.php

Also we really ought to have a test implementation - ie. a test module in the field/tests/modules that provides an implementation that can be tested by a test in the field module

The new hook needs an example in field.api.php

Also we really ought to have a test implementation - ie. a test module in the field/tests/modules that provides an implementation that can be tested by a test in the field module

Not really..

So, the patch is a bit weird because it invokes an already existing hook (!) that is usually invoked by the EntityListController, but since the field list is not yet a list controller, Gabor wants to have this "shortcut" for the config translation module until #1963340: Change field UI so that adding a field is a separate task happens.

The hook added here is actually https://api.drupal.org/api/drupal/core!includes!entity.api.php/function/..., and it's already tested by https://api.drupal.org/api/drupal/core!modules!system!tests!modules!enti...

Thanks for answering my questions @amateescu

Committed faaaaae and pushed to 8.x. Thanks!

Nice git hash.