Follow up for #1971384: [META] Convert page callbacks to controllers and #1913618: Convert EntityFormControllerInterface to extend FormInterface

Whilst we have _entity_form for our defaults in routing, it can't be expected to deal with entity add callbacks, as these need to create a psuedo entity first and the keys they need to set vary (eg menu items may need a plid) or are dynamic (eg node/add/type and block/add/custom_block_type).

This leaves the controller methods for callbacks like block/add/custom_block_type #1978166: Convert block/add/{%custom_block_type} to Controller needing to call entity_get_form from their controllers which breaks our OO goal.

This issue tracks moving entity_get_form to Drupal\Core\Entity\EntityManager::getForm()

Related

#1982984: Create Drupal::entityManager for improved DX

Files: 
CommentFileSizeAuthor
#42 entity-get-form-1982980-38.patch21.86 KBParisLiakos
PASSED: [[SimpleTest]]: [MySQL] 58,092 pass(es).
[ View ]
#42 interdiff.txt2.33 KBParisLiakos
#36 entity-get-form-1982980-36.patch21.98 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]
#36 interdiff.txt2.56 KBParisLiakos
#34 entity-get-form-1982980-43.patch22.54 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#30 entity-get-form-1982980-30.patch18.05 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 58,010 pass(es).
[ View ]
#28 entity-get-form-00-1982980.28.patch18.03 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch entity-get-form-00-1982980.28.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#26 entity-get-form-00-1982980.26.patch18.66 KBParisLiakos
PASSED: [[SimpleTest]]: [MySQL] 57,599 pass(es).
[ View ]
#24 entity-get-form-00-1982980.24.patch19.74 KBParisLiakos
PASSED: [[SimpleTest]]: [MySQL] 56,691 pass(es).
[ View ]
#20 entity-get-form-00-1982980.20.patch20.01 KBParisLiakos
PASSED: [[SimpleTest]]: [MySQL] 58,788 pass(es).
[ View ]
#17 entity-get-form-00-1982980.17.patch20.82 KBParisLiakos
PASSED: [[SimpleTest]]: [MySQL] 56,196 pass(es).
[ View ]
#16 entity-get-form-00-1982980.16.patch20.85 KBParisLiakos
PASSED: [[SimpleTest]]: [MySQL] 55,662 pass(es).
[ View ]
#15 entity-get-form-00-1982980.15.patch20.85 KBParisLiakos
FAILED: [[SimpleTest]]: [MySQL] 55,692 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#12 entity-get-form-00-1982980.12.interdiff.txt1.43 KBlarowlan
#12 entity-get-form-00-1982980.12.patch20.64 KBlarowlan
PASSED: [[SimpleTest]]: [MySQL] 55,533 pass(es).
[ View ]
#10 drupal-1982980-10.patch21.24 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 55,996 pass(es).
[ View ]
#7 drupal-1982980-7.patch21.54 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 55,509 pass(es).
[ View ]
#4 entity-get-form-00-1982980.3.interdiff.txt3.61 KBlarowlan
#4 entity-get-form-00-1982980.3.patch22.16 KBlarowlan
PASSED: [[SimpleTest]]: [MySQL] 55,409 pass(es).
[ View ]
#1 entity-get-form-00-1982980.1.patch22.15 KBlarowlan
FAILED: [[SimpleTest]]: [MySQL] 55,372 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

Comments

Assigned:larowlan» Unassigned
Status:Active» Needs review
StatusFileSize
new22.15 KB
FAILED: [[SimpleTest]]: [MySQL] 55,372 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

How goes it bot?

Status:Needs review» Needs work

The last submitted patch, entity-get-form-00-1982980.1.patch, failed testing.

+++ b/core/modules/options/lib/Drupal/options/Tests/OptionsFieldTest.phpundefined
@@ -65,7 +65,7 @@ function testUpdateAllowedValues() {
+    $form = Drupal::service('plugin.manager.entity')->getForm($entity);
@@ -91,7 +91,7 @@ function testUpdateAllowedValues() {
+    $form = Drupal::service('plugin.manager.entity')->getForm($entity);
@@ -99,7 +99,7 @@ function testUpdateAllowedValues() {
+    $form = Drupal::service('plugin.manager.entity')->getForm($entity);
@@ -120,7 +120,7 @@ function testUpdateAllowedValues() {
+    $form = Drupal::service('plugin.manager.entity')->getForm($entity);
+++ b/core/modules/system/lib/Drupal/system/Tests/Entity/EntityTranslationFormTest.phpundefined
@@ -81,7 +81,7 @@ function testEntityFormLanguage() {
+    Drupal::service('plugin.manager.entity')->getForm($node, 'default', $form_state);

Needs to be \Drupal (that's the two failures)

Status:Needs work» Needs review
StatusFileSize
new22.16 KB
PASSED: [[SimpleTest]]: [MySQL] 55,409 pass(es).
[ View ]
new3.61 KB

yeah, sorry picked that up too
one of those days.

this is awesome:)
+1000

StatusFileSize
new21.54 KB
PASSED: [[SimpleTest]]: [MySQL] 55,509 pass(es).
[ View ]

Updated the patch

Status:Needs review» Needs work

+++ b/core/includes/entity.incundefined
@@ -487,11 +487,12 @@ function entity_form_submit(EntityInterface $entity, $operation = 'default', &$f
function entity_get_form(EntityInterface $entity, $operation = 'default', array $form_state = array()) {

why dont we remove this entirely?

+++ b/core/lib/Drupal/Core/Entity/EntityManager.phpundefined
@@ -200,4 +200,30 @@ public function getAccessController($entity_type) {
+   * @param string $operation
+   *   (optional) The operation identifying the form variation to be returned.
+   * @param array $form_state
+   *   (optional) An associative array containing the current state of the form.
+   *   Use this to pass additional information to the form, such as the
+   *   langcode. Defaults 'default'.

Docs are messed up:) some of documentation for $operation is in $form_state

We cannot remove it until all of the 'page callback' => 'entity_get_form', routes are converted.

Status:Needs work» Needs review
StatusFileSize
new21.24 KB
PASSED: [[SimpleTest]]: [MySQL] 55,996 pass(es).
[ View ]

There are currently 7 different menu entries which still use entity_get_form.

Here is just a simple rerole + fix of the good point in #8.

Issue tags:+Needs reroll

We cannot remove it until all of the 'page callback' => 'entity_get_form', routes are converted.

right i forgot those page callbacks..

this needs a reroll and while at it:

+++ b/core/includes/entity.incundefined
@@ -487,11 +487,12 @@ function entity_form_submit(EntityInterface $entity, $operation = 'default', &$f
+ * @deprecated Use Drupal::entityManager()->getForm() instead
+ *   or _entity_form from a routing.yml file instead of a page callback.

lets fix the double "instead"

StatusFileSize
new20.64 KB
PASSED: [[SimpleTest]]: [MySQL] 55,533 pass(es).
[ View ]
new1.43 KB

Re-roll and fixes #11 as well as a reference to Drupal::service('plugin.manager.entity') that snuck back in.

Status:Needs review» Reviewed & tested by the community
Issue tags:-Needs reroll

Great!

Status:Reviewed & tested by the community» Needs work

No longer applies

curl https://drupal.org/files/entity-get-form-00-1982980.12.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 21135  100 21135    0     0  16854      0  0:00:01  0:00:01 --:--:-- 19587
error: patch failed: core/modules/options/lib/Drupal/options/Tests/OptionsFieldTest.php:65
error: core/modules/options/lib/Drupal/options/Tests/OptionsFieldTest.php: patch does not apply

Status:Needs work» Needs review
StatusFileSize
new20.85 KB
FAILED: [[SimpleTest]]: [MySQL] 55,692 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

reroll, also replaced one more instance is user.admin.inc, probably just sneaked in

StatusFileSize
new20.85 KB
PASSED: [[SimpleTest]]: [MySQL] 55,662 pass(es).
[ View ]

+++ b/core/modules/user/user.admin.incundefined
@@ -479,7 +479,7 @@ function user_admin_roles_list() {
+  return \Drupal::entityManager()->getForm($role);

not needed prefix

StatusFileSize
new20.82 KB
PASSED: [[SimpleTest]]: [MySQL] 56,196 pass(es).
[ View ]

rerolled, replacing more calls that sneaked in the meanwhile

Shouldn't we move "entity_form_state_defaults" to the controller too? Or is that for another issue?

Status:Needs review» Reviewed & tested by the community

Good idea in general, though we don't replace entity_form_submit so far, so let's go with this.

StatusFileSize
new20.01 KB
PASSED: [[SimpleTest]]: [MySQL] 58,788 pass(es).
[ View ]

no longer applies

Status:Reviewed & tested by the community» Needs work
Issue tags:-WSCCI, -FormInterface, -WSCCI-conversion

The last submitted patch, entity-get-form-00-1982980.20.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+WSCCI, +FormInterface, +WSCCI-conversion

#20: entity-get-form-00-1982980.20.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

StatusFileSize
new19.74 KB
PASSED: [[SimpleTest]]: [MySQL] 56,691 pass(es).
[ View ]

chasing HEAD

Status:Reviewed & tested by the community» Needs work

+++ b/core/modules/field/lib/Drupal/field/Plugin/Core/Entity/FieldInstance.phpundefined
@@ -317,7 +317,7 @@ public function save() {
-      $original = \Drupal::service('plugin.manager.entity')
+      $original = \Drupal::entityManager()

Unrelated change.

And the patch no longer applies

curl https://drupal.org/files/entity-get-form-00-1982980.24.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 20213  100 20213    0     0  15323      0  0:00:01  0:00:01 --:--:-- 17871
error: patch failed: core/modules/block/block.admin.inc:84
error: core/modules/block/block.admin.inc: patch does not apply
error: patch failed: core/modules/config/tests/config_test/config_test.module:86
error: core/modules/config/tests/config_test/config_test.module: patch does not apply
error: patch failed: core/modules/node/node.pages.inc:24
error: core/modules/node/node.pages.inc: patch does not apply

Status:Needs work» Needs review
StatusFileSize
new18.66 KB
PASSED: [[SimpleTest]]: [MySQL] 57,599 pass(es).
[ View ]

removed unrelated change and reroll

Status:Needs review» Reviewed & tested by the community

back to rtbc

StatusFileSize
new18.03 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch entity-get-form-00-1982980.28.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

one more reroll after admin/people view went in

Status:Reviewed & tested by the community» Needs work

The last submitted patch, entity-get-form-00-1982980.28.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new18.05 KB
PASSED: [[SimpleTest]]: [MySQL] 58,010 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community
Issue tags:-WSCCI-conversion

we are close to 10 rerolls here:P

Issue tags:+Needs reroll

Ho hum... here we go again...

curl https://drupal.org/files/entity-get-form-1982980-30.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 18484  100 18484    0     0  13193      0  0:00:01  0:00:01 --:--:-- 15454
error: patch failed: core/modules/block/custom_block/custom_block.pages.inc:55
error: core/modules/block/custom_block/custom_block.pages.inc: patch does not apply
error: patch failed: core/modules/node/node.pages.inc:92
error: core/modules/node/node.pages.inc: patch does not apply

Status:Reviewed & tested by the community» Needs work

Status:Needs work» Needs review
StatusFileSize
new22.54 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

conflicted with #1978166: Convert block/add/{%custom_block_type} to Controller
i injected the plugin manager instead and put the request in the controller method since i was touching it

+++ b/core/modules/block/custom_block/lib/Drupal/custom_block/Controller/CustomBlockController.phpundefined
@@ -16,52 +16,29 @@
-    $this->customBlockStorage = $custom_block_storage;
-    $this->customBlockTypeStorage = $custom_block_type_storage;
...
+    $this->entityManager = $entity_manager;
@@ -73,7 +50,7 @@ public function __construct(Request $request, EntityStorageControllerInterface $
-    $types = $this->customBlockTypeStorage->load();
+    $types = $this->entityManager->getStorageController('custom_block_type')->load();

If we need the entity manager in addition, so be it. But this is not the right direction to go in for the storage controllers

StatusFileSize
new2.56 KB
new21.98 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details..
[ View ]

yeap, i stand corrected

Status:Needs review» Needs work
Issue tags:-WSCCI, -Needs reroll, -FormInterface

The last submitted patch, entity-get-form-1982980-36.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+WSCCI, +Needs reroll, +FormInterface

#36: entity-get-form-1982980-36.patch queued for re-testing.

+++ b/core/modules/block/custom_block/lib/Drupal/custom_block/Controller/CustomBlockController.phpundefined
@@ -38,6 +52,8 @@ public static function create(ContainerInterface $container) {
+    $this->customBlockStorage = $entity_manager->getStorageController('custom_block');
+    $this->customBlockTypeStorage = $entity_manager->getStorageController('custom_block_type');

shouldn't we be injecting these too?

+++ b/core/modules/block/custom_block/lib/Drupal/custom_block/Controller/CustomBlockController.phpundefined
@@ -40,28 +40,21 @@ class CustomBlockController implements ControllerInterface {
-      $entity_manager->getStorageController('custom_block'),
-      $entity_manager->getStorageController('custom_block_type')
...
-  public function __construct(Request $request, EntityStorageControllerInterface $custom_block_storage, EntityStorageControllerInterface $custom_block_type_storage) {
-    $this->request = $request;
-    $this->customBlockStorage = $custom_block_storage;
-    $this->customBlockTypeStorage = $custom_block_type_storage;
+  public function __construct(PluginManagerInterface $entity_manager) {
+    $this->customBlockStorage = $entity_manager->getStorageController('custom_block');
+    $this->customBlockTypeStorage = $entity_manager->getStorageController('custom_block_type');
+    $this->entityManager = $entity_manager;

Getting rid of the Request is correct, but not the other two

Status:Needs review» Needs work

The last submitted patch, entity-get-form-1982980-36.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.33 KB
new21.86 KB
PASSED: [[SimpleTest]]: [MySQL] 58,092 pass(es).
[ View ]

k lets just swap request with entity manager

Status:Needs review» Needs work
Issue tags:-WSCCI, -Needs reroll, -FormInterface

The last submitted patch, entity-get-form-1982980-38.patch, failed testing.

Status:Needs work» Needs review

#42: entity-get-form-1982980-38.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, entity-get-form-1982980-38.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+WSCCI, +Needs reroll, +FormInterface

#42: entity-get-form-1982980-38.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community
Issue tags:-Needs reroll

Sixth time's the charm?

Status:Reviewed & tested by the community» Fixed

Committed c4b6658 and pushed to 8.x. Thanks!

I don't think this needs a change notice as we haven't removed entity_get_form() yet...

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue summary:View changes

Updated issue summary.