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
#50 interdiff.txt1.28 KBandypost
#50 taxonomy-1987866-50.patch2.59 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 57,894 pass(es).
[ View ]
#46 interdiff.txt3.21 KBandypost
#46 taxonomy-1987866-45.patch3.44 KBandypost
FAILED: [[SimpleTest]]: [MySQL] 58,055 pass(es), 55 fail(s), and 1 exception(s).
[ View ]
#45 taxonomy-1987866-43.patch3.05 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 58,086 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#45 interdiff.txt1014 bytesdawehner
#41 taxonomy-1987866-41.patch3.04 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]
#38 19876866-taxonomy-add-controller-38.patch1.65 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]
#38 19876866-diff-29-38.txt425 bytesvijaycs85
#29 taxonomy-1987866-29.patch2.47 KBbenjf
PASSED: [[SimpleTest]]: [MySQL] 57,997 pass(es).
[ View ]
#19 taxonomy-1987866-19.patch2.45 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy-1987866-19.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#10 1987866-taxonomy-vocabulary_add_controller-10.patch1.7 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 55,527 pass(es).
[ View ]
#8 1987866-8-taxonomy-vocabulary_add_controller.patch3.28 KBpfrenssen
FAILED: [[SimpleTest]]: [MySQL] 54,970 pass(es), 69 fail(s), and 1 exception(s).
[ View ]
#6 1987866-6-taxonomy-vocabulary_add_controller.patch4.31 KBpfrenssen
FAILED: [[SimpleTest]]: [MySQL] 55,266 pass(es), 69 fail(s), and 1 exception(s).
[ View ]
#4 1987866-4-taxonomy-vocabulary_add_controller.patch4.39 KBpfrenssen
FAILED: [[SimpleTest]]: [MySQL] 55,888 pass(es), 69 fail(s), and 1 exception(s).
[ View ]
#3 1987866-3-taxonomy-vocabulary_add_controller.patch3.37 KBpfrenssen
FAILED: [[SimpleTest]]: [MySQL] 56,267 pass(es), 69 fail(s), and 1 exception(s).
[ View ]

Comments

Assigned:Unassigned» pfrenssen

I'll try my hand at this.

Status:Active» Needs review
StatusFileSize
new3.37 KB
FAILED: [[SimpleTest]]: [MySQL] 56,267 pass(es), 69 fail(s), and 1 exception(s).
[ View ]

Here's a first version. I still have to figure out how to port the access callback. The example does not cover this. Currently the user gets an access denied when visiting admin/structure/taxonomy/add.

StatusFileSize
new4.39 KB
FAILED: [[SimpleTest]]: [MySQL] 55,888 pass(es), 69 fail(s), and 1 exception(s).
[ View ]

Updated version of the patch. Implemented the access check but it is not triggered. User still gets access denied.

Status:Needs review» Needs work

The last submitted patch, 1987866-4-taxonomy-vocabulary_add_controller.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.31 KB
FAILED: [[SimpleTest]]: [MySQL] 55,266 pass(es), 69 fail(s), and 1 exception(s).
[ View ]

Restored the hook_menu() entry and added the route_name to it. Fixed the route_name in the routing.yml file. Still no luck.

Status:Needs review» Needs work

The last submitted patch, 1987866-6-taxonomy-vocabulary_add_controller.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.28 KB
FAILED: [[SimpleTest]]: [MySQL] 54,970 pass(es), 69 fail(s), and 1 exception(s).
[ View ]

Discovered VocabularyAccessController. I'm learning lots of interesting new things here, will get there in the end :)

Status:Needs review» Needs work

The last submitted patch, 1987866-8-taxonomy-vocabulary_add_controller.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.7 KB
PASSED: [[SimpleTest]]: [MySQL] 55,527 pass(es).
[ View ]

Re-roll after #1891690: Use EntityListController for vocabularies landed and updating callback with _entity_form. Changing ACTION_CALLBACK to hook as mentioned in [#2007444] . For access we need to use entity_page_create_access(), however it is not converted to new routing access class. For now setting access as 'administer taxonomy'.

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

The last submitted patch, 1987866-taxonomy-vocabulary_add_controller-10.patch, failed testing.

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

Looks ready

+++ b/core/modules/taxonomy/taxonomy.moduleundefined
@@ -240,13 +240,9 @@ function taxonomy_menu() {
   $items['admin/structure/taxonomy/add'] = array(
     'title' => 'Add vocabulary',
...
+    'route_name' => 'taxonomy_vocabulary_add',
+    'type' => MENU_SIBLING_LOCAL_TASK,

is this still needed?

Status:Needs review» Reviewed & tested by the community

so rtbc

Status:Reviewed & tested by the community» Needs review

You don't need that for the action link itself, but to ensure the local tabs show up on the admin/structure/taxonomy/add itself. They are separate things.

Status:Needs review» Reviewed & tested by the community

Crosspost

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

Needs a reroll

curl https://drupal.org/files/1987866-taxonomy-vocabulary_add_controller-10_0.patch | git apply
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1741  100  1741    0     0   1359      0  0:00:01  0:00:01 --:--:--  1621
error: patch failed: core/modules/taxonomy/taxonomy.routing.yml:5
error: core/modules/taxonomy/taxonomy.routing.yml: patch does not apply

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new2.45 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy-1987866-19.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Rerolled.

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

The last submitted patch, taxonomy-1987866-19.patch, failed testing.

Status:Needs work» Needs review

#19: taxonomy-1987866-19.patch queued for re-testing.

Had this same error earlier today on another issue. After re-test it passed.

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

The last submitted patch, taxonomy-1987866-19.patch, failed testing.

Issue tags:+D8MI

+++ b/core/modules/taxonomy/taxonomy.admin.incundefined
@@ -9,18 +9,6 @@
-  $vocabulary = entity_create('taxonomy_vocabulary', array(
-    // Default the new vocabulary to the site's default language. This is the
-    // most likely default value until we have better flexible settings.
-    'langcode' => language_default()->langcode,
+++ b/core/modules/taxonomy/taxonomy.routing.ymlundefined
@@ -4,3 +4,10 @@ taxonomy_vocabulary_list:
+    _entity_form: 'taxonomy_vocabulary'

We drop this code, is this still needed?

Status:Needs work» Needs review

That line is covered by Term::$values, which is merged in during EntityNG::__construct().

Assigned:pfrenssen» Unassigned

#19: taxonomy-1987866-19.patch queued for re-testing.

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

The last submitted patch, taxonomy-1987866-19.patch, failed testing.

Status:Needs work» Needs review

+++ b/core/modules/taxonomy/taxonomy.moduleundefined
@@ -241,11 +241,8 @@ function taxonomy_menu() {
-    'access callback' => 'entity_page_create_access',
+++ b/core/modules/taxonomy/taxonomy.routing.ymlundefined
@@ -4,3 +4,10 @@ taxonomy_vocabulary_list:
+    _permission: 'administer taxonomy'

I guess it's unlikely that someone wants to alter this through the access controller/hook but still a bit unfortunate to introduce that inconsistency shortly after we managed to make it consistent ;)

We don't have a way to replace entity_page_create_access yet, not sure how to do it. As we optionally need to support the bundle too.

+++ b/core/modules/taxonomy/taxonomy.moduleundefined
@@ -1304,3 +1301,18 @@ function taxonomy_library_info() {
+ * Implements hook_local_actions.

() missing ;)

StatusFileSize
new2.47 KB
PASSED: [[SimpleTest]]: [MySQL] 57,997 pass(es).
[ View ]

patch re-roll attempt:)

I also added the 2nd part of comment #28 above, the missing ().

+++ b/core/modules/taxonomy/taxonomy.routing.ymlundefined
@@ -4,3 +4,10 @@ taxonomy_vocabulary_list:
+    _permission: 'administer taxonomy'

should be _entity_access: taxonomy_vocabulary.add

Assigned:Unassigned» pwieck

Fixing patch #29 to reflex #30

@andypost and @pwieck, may be not for add :( we can't use _entity_access for creating entity.

Assigned:pwieck» Unassigned

NOT fixing patch #29 to reflex #30 per #32...I think?

+++ b/core/modules/taxonomy/taxonomy.moduleundefined
@@ -241,11 +241,8 @@ function taxonomy_menu() {
-    'access callback' => 'entity_page_create_access',
-    'access arguments' => array('taxonomy_vocabulary'),

Well at least for constant bundles it would be possible to support it, but for a dynamic issue like taxonomy_term_create_access has, there is no way around custom implementation.

Here is an issue for that #2028585: Replace entity_page_create_access by an access controller though it is certainly not matching every use case.

Status:Needs review» Needs work

+++ b/core/modules/taxonomy/taxonomy.routing.ymlundefined
@@ -4,3 +4,10 @@ taxonomy_vocabulary_list:
+  requirements:
+    _permission: 'administer taxonomy'

So we should use the new access checker now.

FYI: info about the new access checker is in core/modules/taxonomy/lib/Drupal/taxonomy/Access/TaxonomyTermCreateAccess.php

Status:Needs work» Needs review
Issue tags:+LONDON_2013_JULY
StatusFileSize
new425 bytes
new1.65 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]

Updating patch as per #36 and thanks for reference in #37

Status:Needs review» Needs work

The last submitted patch, 19876866-taxonomy-add-controller-38.patch, failed testing.

Issue tags:+Needs reroll

Status:Needs work» Needs review
Issue tags:-Needs reroll
StatusFileSize
new3.04 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to login to test site.
[ View ]

Let's convert it to a local action plugin.

Status:Needs review» Needs work
Issue tags:-D8MI, -WSCCI-conversion, -MENU_LOCAL_ACTION, -LONDON_2013_JULY

The last submitted patch, taxonomy-1987866-41.patch, failed testing.

Status:Needs work» Needs review

#41: taxonomy-1987866-41.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+D8MI, +WSCCI-conversion, +MENU_LOCAL_ACTION, +LONDON_2013_JULY

The last submitted patch, taxonomy-1987866-41.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1014 bytes
new3.05 KB
FAILED: [[SimpleTest]]: [MySQL] 58,086 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Ups.

StatusFileSize
new3.44 KB
FAILED: [[SimpleTest]]: [MySQL] 58,055 pass(es), 55 fail(s), and 1 exception(s).
[ View ]
new3.21 KB

Suppose better use yml file for local action.
Also access check should use proper implementation

EDIT sorry for xpost
Related #1966436: Default *content* entity languages are not set for entities created with the API

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Plugin/Core/Entity/Vocabulary.php
@@ -25,7 +25,8 @@
- *       "default" = "Drupal\taxonomy\VocabularyFormController",
+ *       "add" = "Drupal\taxonomy\VocabularyFormController",
+ *       "edit" = "Drupal\taxonomy\VocabularyFormController",

Let's exclude this and add the interdiff from #46

Status:Needs review» Needs work

The last submitted patch, taxonomy-1987866-45.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.59 KB
PASSED: [[SimpleTest]]: [MySQL] 57,894 pass(es).
[ View ]
new1.28 KB

Let's see how default actions affects the patch

Status:Needs review» Reviewed & tested by the community

Checked manually worked fine.

Status:Reviewed & tested by the community» Fixed

Committed c534b44 and pushed to 8.x. Thanks!

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