Convert contact_category_list, contact_category_add, and contact_category_edit to new-style Controllers using the instructions on http://drupal.org/node/1800686 . All of those methods can go on the same controller class.

Then, convert the delete form to new-style form controller based on the instructions on the same page.

Related
#1995620: [policy, no patch] Document how to handle routes for MENU_DEFAULT_LOCAL_TASK

Files: 
CommentFileSizeAuthor
#23 contact-1938386-23.patch5.46 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 57,364 pass(es).
[ View ]
#23 interdiff.txt2.91 KBtim.plunkett
#17 interdiff.txt1.45 KBandypost
#17 1938386-contact-route-17.patch4.44 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 55,595 pass(es).
[ View ]
#16 interdiff.txt541 bytesandypost
#16 1938386-contact-route-16.patch4.06 KBandypost
FAILED: [[SimpleTest]]: [MySQL] 56,169 pass(es), 11 fail(s), and 0 exception(s).
[ View ]
#15 1938386-contact-route-15.patch4.03 KBandypost
FAILED: [[SimpleTest]]: [MySQL] 55,492 pass(es), 162 fail(s), and 23 exception(s).
[ View ]
#9 drupal_contact_routing_1938386_9.patch4.43 KBmkadin
FAILED: [[SimpleTest]]: [MySQL] 55,764 pass(es), 169 fail(s), and 39 exception(s).
[ View ]
#2 contact_routing_2.patch4.28 KBmkadin
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]

Comments

Assigned:Unassigned» mkadin

*Sets his crosshairs on this one*

StatusFileSize
new4.28 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]

Trying to figure out what's wrong with the title callback on the edit page.

Status:Active» Needs review

Status:Needs review» Needs work

The last submitted patch, contact_routing_2.patch, failed testing.

I've got a few issues / questions:

1) title_callback isn't being passed an upcasted entity. Looks like defining route_name in your hook_menu() means that that 'path' in the menu system has the symfony style path with entity types enclosed in {}. Perhaps we need to postpone this and starta new issue for dealing with this.

2) Also, I can't seem to figure out the local task piece. Should I be defining two separate routes for /admin/structure/contact/manage/{contact_category}/edit and /admin/structure/contact/manage/{contact_category} ? I dropped the title stuff for now and then tried to figure that out. No configuration (one, the other, or both) led to a result where all of the pages returned content and all of the local task tabs were working properly.

Default local tasks have always required double-entry in the menu system. That hasn't changed at this point.

As discussed in IRC, we probably should hold on this one until we change how the path gets bridged over, and/or let the menu system deal with {} placeholders.

Status:Needs work» Postponed

Status:Postponed» Needs work

As per #7

StatusFileSize
new4.43 KB
FAILED: [[SimpleTest]]: [MySQL] 55,764 pass(es), 169 fail(s), and 39 exception(s).
[ View ]

rerolled, but still not working. On the admin/structure/contact page there's a new error:

Notice: Undefined offset: 4 in _menu_translate() (line 757 of core/includes/menu.inc).

Also going to the edit page for a contact category throws a big ol error. It seems as though #1945418: New-style placeholders in menu_router table break breadcrumbs, menu tree, etc didn't clear up the issue with title arguments / title callbacks.

Heads up, #1983844: Add EntityListController and convert picture.module callbacks to routes/controllers to provide a use-case will add a generic list controller you can use for contact_category_list().

Notice: Undefined offset: 4 in _menu_translate() (line 757 of core/includes/menu.inc).

My temporary fix has been to set tab_root to the actual root. The problem is that there is a single router entry with a different path.

+++ b/core/modules/contact/contact.routing.ymlundefined
@@ -4,3 +4,24 @@ contact_category_delete:
\ No newline at end of file
+++ b/core/modules/contact/lib/Drupal/contact/Controller/ContactController.phpundefined
@@ -0,0 +1,59 @@
\ No newline at end of file

... missing newline

+++ b/core/modules/contact/lib/Drupal/contact/Controller/ContactController.phpundefined
@@ -0,0 +1,59 @@
+use Drupal\Core\ControllerInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+use Drupal\Core\Entity\EntityManager;
+use Drupal\contact\Plugin\Core\Entity\Category;

Let's sort them.

+++ b/core/modules/contact/lib/Drupal/contact/Controller/ContactController.phpundefined
@@ -0,0 +1,59 @@
+class ContactController implements ControllerInterface {

Needs docs.

+++ b/core/modules/contact/lib/Drupal/contact/Controller/ContactController.phpundefined
@@ -0,0 +1,59 @@
+   * The PluginManager for the contact category entity.
+   * @var \Drupal\Core\Entity\EntityManager

There should be an empty line between them.

There's no need in special controller at all

+++ b/core/modules/contact/contact.moduleundefined
@@ -58,28 +58,21 @@ function contact_permission() {
+    'route_name' => 'contact_category_list',
+++ b/core/modules/contact/contact.routing.ymlundefined
@@ -4,3 +4,24 @@ contact_category_delete:
+contact_category_list:
+  pattern: '/admin/structure/contact'
+  defaults:
+    _content: '\Drupal\contact\Controller\ContactController::adminList'
+  requirements:
+    _permission: 'administer contact forms'
+++ b/core/modules/contact/lib/Drupal/contact/Controller/ContactController.phpundefined
@@ -0,0 +1,59 @@
+   * Content for contact category listing page.
+   */
+  public function adminList() {
+    return $this->entityManager->getListController('contact_category')->render();

Should be

      _content: '\Drupal\Core\Entity\Controller\EntityListController::listing'
      entity_type: 'contact_category'

as introduced in #1983844: Add EntityListController and convert picture.module callbacks to routes/controllers to provide a use-case

+++ b/core/modules/contact/lib/Drupal/contact/Controller/ContactController.phpundefined
@@ -0,0 +1,59 @@
+  public function categoryAdd() {
+    $category = entity_create('contact_category', array());
+    return entity_get_form($category);
...
+  public function categoryEdit(Category $contact_category) {
+    return entity_get_form($contact_category);

EntityFormController already have $this->entity so enough in routing

+  defaults:
+    _entity_form: contact_category.default
+  requirements:

Status:Needs work» Needs review
StatusFileSize
new4.03 KB
FAILED: [[SimpleTest]]: [MySQL] 55,492 pass(es), 162 fail(s), and 23 exception(s).
[ View ]

New patch, somehow got Fatal error: Allowed memory size of 536870912 bytes exhausted (tried to allocate 32 bytes) in core8/core/lib/Drupal/Core/Routing/RouteProvider.php on line 122 for list and add

StatusFileSize
new4.06 KB
FAILED: [[SimpleTest]]: [MySQL] 56,169 pass(es), 11 fail(s), and 0 exception(s).
[ View ]
new541 bytes

this fixes memory issue but makes admin/structure/contact/manage/feedback/edit 404 so only admin/structure/contact/manage/feedback works

StatusFileSize
new4.44 KB
PASSED: [[SimpleTest]]: [MySQL] 55,595 pass(es).
[ View ]
new1.45 KB

nasty hack to make it work

Status:Needs review» Needs work
Issue tags:-#SprintWeekend, -WSCCI-conversion, -MENU_LOCAL_ACTION

The last submitted patch, 1938386-contact-route-17.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+#SprintWeekend, +WSCCI-conversion, +MENU_LOCAL_ACTION

#17: 1938386-contact-route-17.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

awesomeness

@andypost
Can you explain why we need two routes even tim's approach on #1995620: [policy, no patch] Document how to handle routes for MENU_DEFAULT_LOCAL_TASK is to use only one?

@dawehner not sure I get how to use 1 route, probably better make it in follow-up

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new2.91 KB
new5.46 KB
PASSED: [[SimpleTest]]: [MySQL] 57,364 pass(es).
[ View ]

Like this.

We should never be adding double routes on purpose.

Also, fixing the double use of default.

Status:Needs review» Reviewed & tested by the community

Awesome idea.

+++ b/core/modules/contact/contact.routing.ymlundefined
@@ -4,3 +4,25 @@ contact_category_delete:
+contact_category_add:
+  pattern: '/admin/structure/contact/add'
+  defaults:
+    _entity_form: contact_category.add
...
+contact_category_edit:
+  pattern: '/admin/structure/contact/manage/{contact_category}'
+  defaults:
+    _entity_form: contact_category.edit
+++ b/core/modules/contact/lib/Drupal/contact/Plugin/Core/Entity/Category.phpundefined
@@ -23,7 +23,8 @@
  *     "form" = {
- *       "default" = "Drupal\contact\CategoryFormController"
+ *       "add" = "Drupal\contact\CategoryFormController",
+ *       "edit" = "Drupal\contact\CategoryFormController"

Would be nice to add to "policy" issue

Status:Reviewed & tested by the community» Fixed

Committed 1d26631 and pushed to 8.x. Thanks!

+++ /dev/nullundefined
@@ -1,53 +0,0 @@
-  drupal_set_title(t('Add contact form category'));
...
-  drupal_set_title(t('Edit %label contact form category', array('%label' => $category->label())), PASS_THROUGH);

it seem that need follow-up for titles

Status:Fixed» Closed (fixed)

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

Status:Closed (fixed)» Needs work

it seem that need follow-up for titles

Oh yeah, probably - and too bad actually.

Status:Needs work» Fixed

Please open a follow-up issue.

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

Issue summary:View changes

added related