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.

CommentFileSizeAuthor
#13 1978972-13-shortcut-controller_conversion.patch10.13 KBpfrenssen
#13 interdiff.txt588 bytespfrenssen
#8 1978972-8-shortcut-controller_conversion.patch9.7 KBpfrenssen
#8 interdiff.txt4.98 KBpfrenssen
#5 1978972-shortcut_link_edit-5.patch7.17 KBAjitS
#3 shortcut_link_edit_1978972.patch7.27 KBdutchyoda
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xtfer’s picture

xtfer CreditAttribution: xtfer commented
Assigned: Unassigned » xtfer
dutchyoda’s picture

dutchyoda
he/him
Primary language Dutch
Location Lelystad
CreditAttribution: dutchyoda commented
Assigned: xtfer » dutchyoda
dutchyoda’s picture

dutchyoda
he/him
Primary language Dutch
Location Lelystad
CreditAttribution: dutchyoda commented
Status: Active » Needs review
FileSize
shortcut_link_edit_1978972.patch7.27 KB

I created a seperate Elements file, so it can be used in both link add and link edit.
Furthermore I made the changes to make everything work by controller.

h3rj4n’s picture

h3rj4n CreditAttribution: h3rj4n commented
Status: Needs review » Needs work 
+++ b/core/modules/shortcut/lib/Drupal/shortcut/Access/LinkEditAccessCheck.php
@@ -0,0 +1,38 @@
+}

There should be an enter at the end (multiple).

+++ b/core/modules/shortcut/lib/Drupal/shortcut/Form/Elements.php
@@ -0,0 +1,69 @@
+  /**
+   * Constructs a BookManager object.

Should be {@inheritdoc} right?

AjitS’s picture

AjitS
he/him
Primary language Marathi
Location Pune
CreditAttribution: AjitS commented
Status: Needs work » Needs review
FileSize
1978972-shortcut_link_edit-5.patch7.17 KB

Made changes according to #4.

AjitS’s picture

AjitS
he/him
Primary language Marathi
Location Pune
CreditAttribution: AjitS commented

Just re-rolled the previous patch with the changes suggested. However, I see that the menu style for admin/config/user-interface/shortcut/link still needs to be changed to new routing system.
Should the status be changed to "needs work"? Or will that part be taken care of in some other issue?

h3rj4n’s picture

h3rj4n CreditAttribution: h3rj4n commented
Status: Needs review » Needs work

No, that should be fixed in this issue. This definitely needs work! ;)

As long as the routing.yml and the hook_menu aren't changed, none of the code of the patch is tested. So that needs to be in this patch.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
FileSize
interdiff.txt4.98 KB
1978972-8-shortcut-controller_conversion.patch9.7 KB

I updated routing.yml and hook_menu(). The access check was identical to the one used for shortcut_link_delete, so rolled them into one.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: -FormInterface

The last submitted patch, 1978972-8-shortcut-controller_conversion.patch, failed testing.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
Status: Needs work » Needs review

#8: 1978972-8-shortcut-controller_conversion.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +FormInterface

The last submitted patch, 1978972-8-shortcut-controller_conversion.patch, failed testing.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
Status: Needs work » Needs review
FileSize
interdiff.txt588 bytes
1978972-13-shortcut-controller_conversion.patch10.13 KB

Status: Needs review » Needs work

The last submitted patch, 1978972-13-shortcut-controller_conversion.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Assigned: dutchyoda » Unassigned
Status: Needs work » Closed (duplicate)

This needs to add/change the same code as #1978966: Convert shortcut_link_add and shortcut_link_edit to a Controller, merging them.