Part of #1971384: [META] Convert page callbacks to controllers

For instructions on how to convert a page callback into a controller, see the WSCCI Conversion Guide.

Files: 
CommentFileSizeAuthor
#41 entity-form-1987692-41.patch8.19 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 58,354 pass(es).
[ View ]
#38 entity-form-1987692-38.patch8.25 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch entity-form-1987692-38.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#34 entity_get_form-1987692-34.patch9.67 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 58,233 pass(es).
[ View ]
#30 drupal8.menu-system.1987692-30.patch9.42 KBbecw
PASSED: [[SimpleTest]]: [MySQL] 58,329 pass(es).
[ View ]
#30 drupal8.menu-system.1987692-30.interdiff.txt1.24 KBbecw
#29 drupal8.menu-system.1987692-29.patch9.88 KBbecw
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#29 drupal8.menu-system.1987692-29.interdiff.txt1.55 KBbecw
#27 convert-entity_get_form-to-routing-1987692-25.patch9.6 KBbecw
FAILED: [[SimpleTest]]: [MySQL] 57,871 pass(es), 49 fail(s), and 14 exception(s).
[ View ]
#25 convert-entity_get_form-to-routing-1987692-25.patch9.6 KBbecw
FAILED: [[SimpleTest]]: [MySQL] 58,118 pass(es), 49 fail(s), and 14 exception(s).
[ View ]
#25 convert-entity_get_form-to-routing-1987692-25-interdiff.patch3.57 KBbecw
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch convert-entity_get_form-to-routing-1987692-25-interdiff.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#23 convert-entity_get_form-to-routing-1987692-23.patch9.68 KBbecw
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch convert-entity_get_form-to-routing-1987692-23.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#23 convert-entity_get_form-to-routing-1987692-23-interdiff.txt3.34 KBbecw
#22 convert-entity_get_form-to-routing-1987692-22.patch8.59 KBbecw
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch convert-entity_get_form-to-routing-1987692-22.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#19 convert-entity_get_form-to-routing-1987692-19.patch7.8 KBbecw
FAILED: [[SimpleTest]]: [MySQL] 57,948 pass(es), 49 fail(s), and 8 exception(s).
[ View ]
#18 convert-entity_get_form-to-routing-1987692-18.patch3.46 KBbecw
FAILED: [[SimpleTest]]: [MySQL] 58,088 pass(es), 44 fail(s), and 7 exception(s).
[ View ]
#15 convert-entity_get_form-to-routing-1987692-15.patch10.72 KBcbiggins
PASSED: [[SimpleTest]]: [MySQL] 55,754 pass(es).
[ View ]
#15 1987692-inderdiff.txt6.76 KBcbiggins
#12 convert-entity_get_form-to-routing-1987692-12.patch6.29 KBcbiggins
FAILED: [[SimpleTest]]: [MySQL] 55,815 pass(es), 57 fail(s), and 19 exception(s).
[ View ]
#8 convert-entity_get_form-to-routing-1987692-7.patch8.25 KBcbiggins
FAILED: [[SimpleTest]]: [MySQL] 55,841 pass(es), 119 fail(s), and 21 exception(s).
[ View ]

Comments

Assigned:Unassigned» cbiggins

Using #1978946: Convert comment_edit_page() to a Controller as an example here. (And adding this comment as a reminder to myself.)

Status:Active» Closed (won't fix)

entity_get_form() is not a form controller, it's just a procedural helper :) And it's going away in #1982980: Move entity_get_form to Drupal\Core\Entity\EntityManager::getForm() anyway..

Yep, been chatting with larowlan about this today.

Hi,

Since you guys seem to know what's going on on the entity-side of things, could this issue be of the same type:

#1987698 : Convert entity_view() to a new style controller

As with entity_get_form, entity_view is in entity.inc

Any ideas ?
Just looking to see if i can help a hand on conversion issues, preferably clear ones that need to be done :)

Yeah, that's the same situation. Thanks for bringing it up :)

Title:Convert entity_get_form() to a new style controllerConvert all references to entity_get_form() as page callbacks to _entity_form w/ corresponding routing.yml entries
Component:entity system» menu system
Status:Closed (won't fix)» Active

Even though entity_get_form is going OO
We still need to convert all entries in hook_menu where the page callback is entity_get_form to use _entity_form and routing.
Updating issue title.

Component:menu system» entity system
Status:Active» Closed (won't fix)
StatusFileSize
new8.25 KB
FAILED: [[SimpleTest]]: [MySQL] 55,841 pass(es), 119 fail(s), and 21 exception(s).
[ View ]

Patch attached.

Component:entity system» menu system
Status:Closed (won't fix)» Needs review

Woops, fixing statuses.

Status:Needs review» Needs work

The last submitted patch, convert-entity_get_form-to-routing-1987692-7.patch, failed testing.

+++ b/core/modules/aggregator/aggregator.routing.ymlundefined
@@ -53,3 +53,17 @@ aggregator_page_last:
+  pattern: 'aggregator/sources/{aggregator_feed}/configure'
...
+  pattern: 'admin/config/services/aggregator/edit/feed/{aggregator_feed}'
+++ b/core/modules/menu/menu.routing.ymlundefined
@@ -25,3 +25,10 @@ menu_delete_menu:
+  pattern: 'admin/structure/menu/item/{menu_link}/edit'
+++ b/core/modules/taxonomy/taxonomy.routing.ymlundefined
@@ -0,0 +1,13 @@
+  pattern: 'taxonomy/term/{taxonomy_term}/edit'
...
+  pattern: 'admin/structure/taxonomy/manage/{taxonomy_vocabulary}/edit'
+++ b/core/modules/user/user.routing.ymlundefined
@@ -54,3 +54,10 @@ user_role_delete:
+  pattern: 'user/{user}/edit'

needs leading /

+++ b/core/modules/aggregator/aggregator.routing.ymlundefined
--- /dev/null
+++ b/core/modules/entity/lib/Drupal/entity/Controller/EntityController.phpundefined
+++ b/core/modules/entity/lib/Drupal/entity/Controller/EntityController.phpundefined
+++ b/core/modules/entity/lib/Drupal/entity/Controller/EntityController.phpundefined
@@ -0,0 +1,56 @@
@@ -0,0 +1,56 @@
+<?php
+/**
+* @file
+* Contains \Drupal\entity\Controller\EntityController.
+*/
+
+namespace Drupal\entity\Controller;
+
+use Drupal\Core\ControllerInterface;
+use Drupal\Core\Entity\EntityInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
+/**
+* Controller routines for entity routes.
+*/
+class EntityController implements ControllerInterface {
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function create(ContainerInterface $container) {
+    return new static();
+  }
+
+  /**
+   * Constructs a EntityController object.
+   */
+  public function __construct() {
+  }
+
+  /**
+   * Returns the built and processed entity form for the given entity.
+   *
+   * @param EntityInterface $entity
+   *   The entity to be created or edited.
+   * @param $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.
+   *   @code
+   *   $form_state['langcode'] = $langcode;
+   *   $form = entity_get_form($entity, 'default', $form_state);
+   *   @endcode
+   *
+   * @return
+   *   The processed form for the given entity and operation.
+   */
+  public function getForm(EntityInterface $entity, $operation = 'default', array $form_state = array()) {
+    var_dump(get_class($entity)); exit;
+    $form_state += entity_form_state_defaults($entity, $operation);
+    $form_id = $form_state['build_info']['callback_object']->getFormID();
+    return drupal_build_form($form_id, $form_state);
+  }
+
+}

this looks like left-over from another patch?

+++ b/core/modules/taxonomy/taxonomy.routing.ymlundefined
@@ -0,0 +1,13 @@
+    _entity_form: taxonomy_term.default
...
+    _entity_access: taxonomy_term.update
...
+    _entity_form: taxonomy_vocabulary.default
...
+    _entity_access: taxonomy_vocabulary.update
+++ b/core/modules/user/user.routing.ymlundefined
@@ -54,3 +54,10 @@ user_role_delete:
+    _entity_form: user.profile
...
+    _entity_access: user.update

I think you need to single quote these?

+++ b/core/modules/taxonomy/taxonomy.routing.ymlundefined
@@ -0,0 +1,13 @@
\ No newline at end of file

needs a new line

The fails are down to missing include files. eg the cancel form is in the user.pages.inc but we're not loading that now. Those forms should really become their own page callbacks, there is one for the vocabulary one already, not sure about the user cancel.

+++ b/core/modules/menu/menu.routing.ymlundefined
@@ -25,3 +25,10 @@ menu_delete_menu:
+    _entity_form: 'menu.default'

should be menu_link.default - will fix a swathe of the fails

StatusFileSize
new6.29 KB
FAILED: [[SimpleTest]]: [MySQL] 55,815 pass(es), 57 fail(s), and 19 exception(s).
[ View ]

this looks like left-over from another patch?

Woops, I started writing Controllers in a typical approach to WSCCI conversion. I forgot to delete it before I created a patch. :)

I think you need to single quote these?

I had them single quoted but found a whole bunch of instances where they weren't quoted (user.routing.yml for example.) So I assumed that they shouldn't be. Fixed now.

should be menu_link.default - will fix a swathe of the fails

Sweet!

Thanks larowlan!

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, convert-entity_get_form-to-routing-1987692-12.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new6.76 KB
new10.72 KB
PASSED: [[SimpleTest]]: [MySQL] 55,754 pass(es).
[ View ]

Status:Needs review» Postponed

looks like a great cleanup:)
lets wait for #1946456: Convert taxonomy_term_confirm_delete() and taxonomy_vocabulary_confirm_delete() to the new form interface thats RTBC so we can get rid of the @todo as well

Status:Postponed» Needs work

Assigned:cbiggins» becw
StatusFileSize
new3.46 KB
FAILED: [[SimpleTest]]: [MySQL] 58,088 pass(es), 44 fail(s), and 7 exception(s).
[ View ]

The patch in #15 is very out of date. I'm going to steal this issue and re-rolling the patch; here is just the user edit portion. More coming shortly.

Assigned:becw» Unassigned
Status:Needs work» Needs review
StatusFileSize
new7.8 KB
FAILED: [[SimpleTest]]: [MySQL] 57,948 pass(es), 49 fail(s), and 8 exception(s).
[ View ]

The original patch had routes for aggregator feed, menu link, taxonomy term and vocabulary, and user entity edit forms, but the menu link route updates have already been applied as part of some other issue.

Here's an updated patch that updates the remaining uses of entity_get_form() in hook_menu().

Wow, thanks for jumping on this!

  1. +++ b/core/modules/user/lib/Drupal/user/ProfileFormController.php
    @@ -41,4 +41,19 @@ public function save(array $form, array &$form_state) {
    +   * Submit method for the 'Cancel account' button.

    Technically this should be something like "Provides a submit handler for the 'Cancel account' button."

  2. +++ b/core/modules/user/lib/Drupal/user/ProfileFormController.php
    @@ -41,4 +41,19 @@ public function save(array $form, array &$form_state) {
    +    $query = Drupal::request()->query;

    We should make the buildForm store the request, and reuse it here.

  3. +++ b/core/modules/user/lib/Drupal/user/ProfileFormController.php
    @@ -41,4 +41,19 @@ public function save(array $form, array &$form_state) {
    +    $account = $form_state['controller']->getEntity();
    +    $form_state['redirect'] = array("user/" . $account->id() . "/cancel", array('query' => $destination));

    Instead of all this to get $account, you can just use $this->entity, that pattern was for external code. Also, can we swap the double quotes for single quotes?

Status:Needs review» Needs work

The last submitted patch, convert-entity_get_form-to-routing-1987692-19.patch, failed testing.

Assigned:Unassigned» becw
StatusFileSize
new8.59 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch convert-entity_get_form-to-routing-1987692-22.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

I've made the changes you suggested, and I also took the opportunity to replace $GLOBAL['user'] with $this->request->attributes->get('_account');.

Testbot reported some issues too, but I haven't fixed those yet. I'll leave this as "needs work" until I finish addressing those.

Status:Needs work» Needs review
StatusFileSize
new3.34 KB
new9.68 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch convert-entity_get_form-to-routing-1987692-23.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

The failing tests are at least in part due to issues with the tests themselves:

  • Drupal\taxonomy\Tests\TermTest includes a test that ensures that entity_get_form() does not accept additional arguments from the URL. This test isn't relevant anymore since the new routing system doesn't route these paths to the term edit page:

    <?php
       
    /* Drupal\taxonomy\Tests\TermTest, lines 351-354 */
        // Check that the term edit page does not try to interpret additional path
        // components as arguments for entity_get_form().
       
    $this->drupalGet('taxonomy/term/' . $term->id() . '/edit/' . $this->randomName());
       
    $this->assertResponse(200, 'The taxonomy term edit menu item ensured appropriate arguments were passed to its page callback.');
    ?>
  • Drupal\user\Tests\UserCancelTest's testUserCancelInvalid() seems to have a bug where it doesn't confirm the account cancellation correctly.

Additionally, the new request dependency in ProfielFormController was missing the "use" statement at the top of the file.

Here's an updated patch, with an interdiff to the patch from the re-roll in #19.

Status:Needs review» Needs work

The last submitted patch, convert-entity_get_form-to-routing-1987692-23.patch, failed testing.

Assigned:becw» Unassigned
Status:Needs work» Needs review
StatusFileSize
new3.57 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch convert-entity_get_form-to-routing-1987692-25-interdiff.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new9.6 KB
FAILED: [[SimpleTest]]: [MySQL] 58,118 pass(es), 49 fail(s), and 14 exception(s).
[ View ]

There were some changes to aggregator.module in the past few hours. Here's a re-roll, again with an interdiff against the patch in #19.

Status:Needs review» Needs work

The last submitted patch, convert-entity_get_form-to-routing-1987692-25-interdiff.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new9.6 KB
FAILED: [[SimpleTest]]: [MySQL] 57,871 pass(es), 49 fail(s), and 14 exception(s).
[ View ]

I get different test failures when I run these tests locally, so I'm not quite sure what's going on. I'm going to post the same patch once more, without the mis-named interdiff.

Status:Needs review» Needs work

The last submitted patch, convert-entity_get_form-to-routing-1987692-25.patch, failed testing.

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

Here's an update that fixes dependency injection now that FormBase is in (#2059245: Add a FormBase class containing useful methods).

StatusFileSize
new1.24 KB
new9.42 KB
PASSED: [[SimpleTest]]: [MySQL] 58,329 pass(es).
[ View ]

timplunkett pointed out in IRC that this should use FormBase::getCurrentUser().

Status:Needs review» Reviewed & tested by the community

This looks perfect now. Thanks so much @becw!

grep -r "entity_get_form[^_]" *
core/includes/entity.inc: *   $form = entity_get_form($entity, 'default', $form_state);
core/includes/entity.inc:function entity_get_form(EntityInterface $entity, $operation = 'default', array $form_state = array()) {

Note, this has the same hunk that is being discussed in #1987898-40: Convert user_view_page() to a new style controller

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs reroll

Patch no longer applies

Status:Needs work» Reviewed & tested by the community
Issue tags:-Needs reroll
StatusFileSize
new9.67 KB
PASSED: [[SimpleTest]]: [MySQL] 58,233 pass(es).
[ View ]

Straight reroll.

Issue tags:+Needs reroll

Patch no longer applies

Status:Reviewed & tested by the community» Needs work

Poor @tim.plunkett :)

Status:Needs work» Reviewed & tested by the community
Issue tags:-Needs reroll
StatusFileSize
new8.25 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch entity-form-1987692-38.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Issue tags:-WSCCI-conversion

#38: entity-form-1987692-38.patch queued for re-testing.

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

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

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new8.19 KB
PASSED: [[SimpleTest]]: [MySQL] 58,354 pass(es).
[ View ]

Patch context conflict, nothing more.

Status:Reviewed & tested by the community» Fixed

Comitted and pushed to 8.x. Thanks!

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