Convert shortcut_link_add and shortcut_link_edit to a Controller

Here is the first patch for this issue. Please comment to make it better.

A new patch. making some correction in controller class.

Blocks #1908756: Separate Action Links (MENU_LOCAL_ACTION) from hook_menu()

thanks:)

+++ b/core/modules/shortcut/lib/Drupal/shortcut/Access/LinkAddAccessCheck.phpundefined
@@ -0,0 +1,36 @@
+class linkAddAccessCheck implements AccessCheckInterface {

class name should be uppercase first

+++ b/core/modules/shortcut/lib/Drupal/shortcut/Form/LinkAdd.phpundefined
@@ -0,0 +1,166 @@
+      $shortcut_link = entity_create('menu_link', array(

lets inject entity manager, like you did for module handler,

+++ b/core/modules/shortcut/lib/Drupal/shortcut/Form/LinkAdd.phpundefined
@@ -0,0 +1,166 @@
+      $shortcut_link['link_path'] = ($shortcut_link['link_path'] == '<front>') ? '' : drupal_container()->get('path.alias_manager')->getPathAlias($shortcut_link['link_path']);

same for path alias manager

+++ b/core/modules/shortcut/lib/Drupal/shortcut/Form/LinkAdd.phpundefined
@@ -0,0 +1,166 @@
+    $this->database = $database;
+    $this->moduleHandler = $module_handler;

i cant see them used anywhere, you should inject the two managers i pointed above instead

The patch fails at "patch failed: core/modules/shortcut/shortcut.routing.yml:15"
The line is the route name shortcut_set_admin. why it is failing ?

maybe apply manually the changes to routing file and reroll?

#12: shortcut-link-add-1978966-12.patch queued for re-testing.

+++ b/core/modules/shortcut/lib/Drupal/shortcut/Form/LinkAdd.phpundefined
@@ -0,0 +1,139 @@
+    shortcut_admin_add_link($shortcut_link, $this->shortcut);

+++ b/core/modules/shortcut/shortcut.admin.incundefined
@@ -426,45 +399,6 @@ function shortcut_link_edit_submit($form, &$form_state) {
-function shortcut_admin_add_link($shortcut_link, &$shortcut_set) {

+++ b/core/modules/shortcut/shortcut.moduleundefined
@@ -582,3 +573,29 @@ function shortcut_library_info() {
+function shortcut_admin_add_link($shortcut_link, &$shortcut_set) {

i dont think you should move this function to the module file..instead in the controller, you can loadInclude the file using the module_handler service (injected or by \Drupal::moduleHandler() with an @todo above it)

+++ b/core/modules/shortcut/lib/Drupal/shortcut/Form/LinkAdd.phpundefined
@@ -0,0 +1,139 @@
+  public function _shortcut_link_form_elements($shortcut_link = NULL) {

this needs a better naming a la OOP and visibility to protected eg, protected funtion linkFormElements

working on this..

This might not be correct but the 'add shortcut' link on /index.php/admin/config/user-interface/shortcut/manage/default went missing after changes as in the interdiff.

+++ b/core/modules/shortcut/lib/Drupal/shortcut/Access/ShortcutLinkAccessCheck.phpundefined
@@ -0,0 +1,52 @@
+            return $shortcut == shortcut_current_displayed_set();

it would be nice to also return static::ALLOW or static::DENY

+++ b/core/modules/shortcut/lib/Drupal/shortcut/Form/LinkAdd.phpundefined
@@ -0,0 +1,150 @@
+   * @param $shortcut_link
...
+  public function getFormElements($shortcut_link = NULL) {

Let's typehint this variable.

+++ b/core/modules/shortcut/lib/Drupal/shortcut/Form/LinkAdd.phpundefined
@@ -106,8 +106,8 @@ public function submitForm(array &$form, array &$form_state) {
+  public function getFormElements(array $shortcut_link = array()) {

Is this really an array? I thought it was a MenuLinkInterface, but MenuLink just uses ArrayAccess...

Working on this, and merging with #1978972: Convert shortcut_link_edit to a Controller

Posting some progress for now, does not include edit yet.

+++ b/core/modules/shortcut/lib/Drupal/shortcut/Form/ShortcutLinkAddForm.phpundefined
@@ -0,0 +1,107 @@
+    if (!shortcut_valid_link($form_state['values']['shortcut_link']['link_path'])) {
...
+    shortcut_admin_add_link($shortcut_link, $this->entity);

Can we create shortcutManger for these helper methods and inject manger?

+++ b/core/modules/shortcut/shortcut.moduleundefined
@@ -116,12 +117,8 @@ function shortcut_menu() {
     'type' => MENU_LOCAL_ACTION,

Are we going to handle this in this issue or follow up?

+++ b/core/modules/shortcut/shortcut.routing.ymlundefined
@@ -33,6 +33,13 @@ shortcut_set_edit:
+    _entity_access: 'shortcut_set.update'

update doesn't seems right it should be add/create

I will see what's left of those functions when I finish refactoring, same for local action/tasks

It's add/create of the *shortcut link*, which is currently just a menu link. But the access checks are 100% identical to updating a *shortcut set*

Doing something with shortcut_valid_link() should be a follow-up.

Until the local action/task APIs are fixed, not bothering with that either.

Tested on simpletest.me works fine. @tim.plunkett awesome work turning _shortcut_link_form_elements to a ShortcutLinkFormBase. RTBC if it is green.

+++ b/core/modules/shortcut/lib/Drupal/shortcut/Controller/ShortcutSetController.phpundefined
@@ -94,7 +94,7 @@ public function addShortcutLinkInline(ShortcutSetInterface $shortcut_set, Reques
       $this->moduleHandler->loadInclude('shortcut', 'admin.inc');

This can be removed now and we can eject moduleHandler from the controller as well.

Went through and cleaned everything up, including removing unneeded services, and injecting the entity manager into the access check.

Thanks for the clean up.

This patch adds a button on admin/config/user-interface/shortcut which links to admin/config/user-interface/shortcut/add-set which gets the following error...

InvalidArgumentException: The entity type (shortcut_set) did not specify a form controller: add. in Drupal\Core\Entity\EntityManager->getControllerClass() (line 170 of /Volumes/devdisk/dev/sites/drupal8alt.dev/core/lib/Drupal/Core/Entity/EntityManager.php).

However the add set from the profile account tab is still working.

Additionally, on admin/config/user-interface/shortcut each shortcut set has an "edit menu" operation which is weird as it goes to the edit shortcut set name page...

Postponing on #2021779: Decouple shortcuts from menu links anyway.

Updated: Comment #32

Problem/Motivation

Convert shortcut_link_add and shortcut_link_edit to new-style Controllers. To reproduce:

• Install a clean instance of D8.
• Verify on admin/config/user-interface/shortcut that there is no "Add Shortcut Set" button

Proposed resolution

Use the instructions on https://drupal.org/node/1953342.

Remaining tasks

The following issues remain:

#1: When visiting admin/config/user-interface/shortcut/add-set, the following error appears:

InvalidArgumentException: The entity type (shortcut_set) did not specify a form controller: add. in Drupal\Core\Entity\EntityManager->getControllerClass() (line 170 of /Users/luke/Sites/d8core.dev/www/core/lib/Drupal/Core/Entity/EntityManager.php).

#2: When listing links (eg: admin/config/user-interface/shortcut/manage/default) or editing a shortcut set (eg: admin/config/user-interface/shortcut/manage/default/edit) the following error appears:

Warning: call_user_func_array() expects parameter 1 to be a valid callback, function 'shortcut_set_edit_access' not found or invalid function name in _menu_check_access() (line 602 of core/includes/menu.inc).

#3: When attempting to edit or delete a shortcut from a set, I get partially-rendered page with no errors (but am unable to edit or delete the link). From my Apache error log:

PHP Fatal error: Class 'Drupal\\shortcut\\Access\\LinkDeleteAccessCheck' not found in /Users/luke/Sites/d8core.dev/www/sites/default/files/php/service_container/service_container_prod.php/8fc78707b0474cc1c72a8e089285931009388c205d990b9acf87cef0625d5fd1.php on line 482, referer: http://d8core.dev/admin/config/user-interface/shortcut/manage/default

Related Issues

Part of #1971384: [META] Convert page callbacks to controllers

Here is a re-rolled patch. None of the issues in the summary are addressed in this patch.

This issue is postponed because the shortcuts are slated to be completely rearchitected. The work I did (which you rerolled, thanks!) is probably useless...

Updating this following the issue summary template.

Can we close it as duplicate?

Dupe of #2021779: Decouple shortcuts from menu links