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

tim.plunkett’s picture

pfrenssen’s picture

Assigned: Unassigned » pfrenssen

I'll try my hand at this.

pfrenssen’s picture

Status: Active » Needs review
FileSize
3.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.

pfrenssen’s picture

FileSize
4.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.

pfrenssen’s picture

Status: Needs work » Needs review
FileSize
4.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.

pfrenssen’s picture

Status: Needs work » Needs review
FileSize
3.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.

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
1.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.

vijaycs85’s picture

Status: Needs work » Needs review
Issue tags: +WSCCI-conversion, +MENU_LOCAL_ACTION
andypost’s picture

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?

jibran’s picture

andypost’s picture

Status: Needs review » Reviewed & tested by the community

so rtbc

tim.plunkett’s picture

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.

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

Crosspost

alexpott’s picture

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
tim.plunkett’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2.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.

pwieck’s picture

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.

andypost’s picture

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?

tim.plunkett’s picture

Status: Needs work » Needs review

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

pfrenssen’s picture

Assigned: pfrenssen » Unassigned
Berdir’s picture

#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.

Berdir’s picture

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 ;)

benjf’s picture

FileSize
2.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 ().

andypost’s picture

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

should be _entity_access: taxonomy_vocabulary.add

pwieck’s picture

Assigned: Unassigned » pwieck

Fixing patch #29 to reflex #30

vijaycs85’s picture

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

pwieck’s picture

Assigned: pwieck » Unassigned

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

dawehner’s picture

+++ 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.

andypost’s picture

dawehner’s picture

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.

mtift’s picture

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

vijaycs85’s picture

Status: Needs work » Needs review
Issue tags: +LONDON_2013_JULY
FileSize
425 bytes
1.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.

jair’s picture

Issue tags: +Needs reroll
dawehner’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
3.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.

jibran’s picture

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.

dawehner’s picture

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

Ups.

andypost’s picture

FileSize
3.44 KB
FAILED: [[SimpleTest]]: [MySQL] 58,055 pass(es), 55 fail(s), and 1 exception(s). View
3.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

jibran’s picture

+++ 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

andypost’s picture

Status: Needs review » Needs work

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

andypost’s picture

Status: Needs work » Needs review
FileSize
2.59 KB
PASSED: [[SimpleTest]]: [MySQL] 57,894 pass(es). View
1.28 KB

Let's see how default actions affects the patch

jibran’s picture

Status: Needs review » Reviewed & tested by the community

Checked manually worked fine.

alexpott’s picture

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.