Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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.
Comment | File | Size | Author |
---|---|---|---|
#50 | interdiff.txt | 1.28 KB | andypost |
#50 | taxonomy-1987866-50.patch | 2.59 KB | andypost |
#46 | interdiff.txt | 3.21 KB | andypost |
#46 | taxonomy-1987866-45.patch | 3.44 KB | andypost |
#45 | taxonomy-1987866-43.patch | 3.05 KB | dawehner |
Comments
Comment #1
tim.plunkettBlocks #1908756: Separate Action Links (MENU_LOCAL_ACTION) from hook_menu()
Comment #2
pfrenssenI'll try my hand at this.
Comment #3
pfrenssenHere'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.
Comment #4
pfrenssenUpdated version of the patch. Implemented the access check but it is not triggered. User still gets access denied.
Comment #6
pfrenssenRestored the hook_menu() entry and added the route_name to it. Fixed the route_name in the routing.yml file. Still no luck.
Comment #8
pfrenssenDiscovered VocabularyAccessController. I'm learning lots of interesting new things here, will get there in the end :)
Comment #10
vijaycs85Re-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'.
Comment #12
vijaycs85#10: 1987866-taxonomy-vocabulary_add_controller-10.patch queued for re-testing.
Comment #13
andypostLooks ready
is this still needed?
Comment #14
jibranyes see Action links are provided by hook_local_actions() instead of MENU_LOCAL_ACTION in hook_menu()
Comment #15
andypostso rtbc
Comment #16
tim.plunkettYou 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.
Comment #17
tim.plunkettCrosspost
Comment #18
alexpottNeeds a reroll
Comment #19
tim.plunkettRerolled.
Comment #21
pwieck CreditAttribution: pwieck commented#19: taxonomy-1987866-19.patch queued for re-testing.
Had this same error earlier today on another issue. After re-test it passed.
Comment #23
andypostWe drop this code, is this still needed?
Comment #24
tim.plunkettThat line is covered by Term::$values, which is merged in during EntityNG::__construct().
Comment #25
pfrenssenComment #26
Berdir#19: taxonomy-1987866-19.patch queued for re-testing.
Comment #28
BerdirI 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.
() missing ;)
Comment #29
benjf CreditAttribution: benjf commentedpatch re-roll attempt:)
I also added the 2nd part of comment #28 above, the missing ().
Comment #30
andypostshould be _entity_access: taxonomy_vocabulary.add
Comment #31
pwieck CreditAttribution: pwieck commentedFixing patch #29 to reflex #30
Comment #32
vijaycs85@andypost and @pwieck, may be not for add :( we can't use _entity_access for creating entity.
Comment #33
pwieck CreditAttribution: pwieck commentedNOT fixing patch #29 to reflex #30 per #32...I think?
Comment #34
dawehnerWell 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.
Comment #35
andypostRelated #1987860: Convert taxonomy_term_add() to a new style controller
Comment #36
dawehnerSo we should use the new access checker now.
Comment #37
mtiftFYI: info about the new access checker is in core/modules/taxonomy/lib/Drupal/taxonomy/Access/TaxonomyTermCreateAccess.php
Comment #38
vijaycs85Updating patch as per #36 and thanks for reference in #37
Comment #40
jair CreditAttribution: jair commentedComment #41
dawehnerLet's convert it to a local action plugin.
Comment #43
jibran#41: taxonomy-1987866-41.patch queued for re-testing.
Comment #45
dawehnerUps.
Comment #46
andypostSuppose 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
Comment #47
jibranLet's exclude this and add the interdiff from #46
Comment #48
andypost@jibran this needed, see #2006348: Remove default/fallback entity form operation
Comment #50
andypostLet's see how default actions affects the patch
Comment #51
jibranChecked manually worked fine.
Comment #52
alexpottCommitted c534b44 and pushed to 8.x. Thanks!