Add a shortcut block view

Thanks @jibran. Will try it out, but from looking at it looks great so far!

1. +++ b/core/modules/shortcut/config/schema/shortcut.views.schema.yml
   @@ -0,0 +1,8 @@
   +# Schema for the views plugins of the Shortcut module.
   +
   +views.argument_default.current_shortcut_set:
   +  type: sequence
   +  label: 'Shortcut set assigned to current user'
   +  sequence:
   +    type: string
   +    label: 'Shortcut set'
   
   +++ b/core/modules/shortcut/src/Plugin/views/argument_default/CurrentShortcutSet.php
   @@ -0,0 +1,45 @@
   + *
   + * This plugin actually has no options so it does not need to do a great deal.
   + *
   

   That comment seems a bit pointless, if you ask me, but the actual thing is that you should not need config schema in case there is no config ... it should inherit things automatically.

2. +++ b/core/modules/shortcut/src/Entity/Shortcut.php
   @@ -25,6 +25,7 @@
     *     "access" = "Drupal\shortcut\ShortcutAccessControlHandler",
   + *     "views_data" = "Drupal\views\EntityViewsData",
     *     "form" = {
   

   OOH, yes! <3

3. +++ b/core/modules/shortcut/src/Plugin/views/argument_default/CurrentShortcutSet.php
   @@ -0,0 +1,45 @@
   + * Default argument plugin to extract the current shortcut
   

   .

4. +++ b/core/modules/shortcut/src/Plugin/views/argument_default/CurrentShortcutSet.php
   @@ -0,0 +1,45 @@
   +  public function getArgument() {
   

   {@inheritdoc}

5. +++ b/core/modules/shortcut/tests/src/Functional/ShortcutBlockTest.php
   @@ -0,0 +1,55 @@
   +class ShortcutBlockTest extends BrowserTestBase {
   

   Oh wow, this actually works?

6. +++ b/core/modules/simpletest/src/BrowserTestBase.php
   @@ -1339,4 +1339,64 @@ protected function drupalUserIsLoggedIn(UserInterface $account) {
   +  protected function drupalPlaceBlock($plugin_id, array $settings = array()) {
   

   Would it be possible to move the existing code to a trait instead?

And a manual test - works pretty nicely.

I was even able to edit the view, switch from HTML list to grid.

+++ b/core/modules/shortcut/config/optional/views.view.shortcuts.yml
@@ -0,0 +1,280 @@
+description: Shortcuts

nitpick: All the other view descriptions end with a full stop. This probably should too.

+++ b/core/modules/shortcut/config/optional/views.view.shortcuts.yml
@@ -0,0 +1,280 @@
+      cache:
+        type: none
+        options: {  }

Let's cache the view ...

+ cacheable: false

There are a couple instances of this. Not sure if it matters.

Just did another manual test. Still works!

Does the cacheable thing matter?

No? Let's RTBC this.
Yes? Back to needs work for a tidy-up.

:)

Nice work everyone.

+++ b/core/modules/shortcut/config/optional/views.view.shortcuts.yml
@@ -0,0 +1,282 @@
+description: 'Shortcuts block.'

Yes. That's better. Just "Shortcuts." had been bugging me.

Do we need a change record for this?

looks good to me

Great! RTBC++

Sorry for jumping in on the issue :) Just fixed some inconsistency on the array syntaxes and documentation

Tested again. Still works!

And in the meantime, the unrelated bug I found while doing initial testing for this has been fixed and committed too! :-)
#2511024: Can't add multiple content types to shortcuts

1. +++ b/core/modules/shortcut/config/optional/views.view.shortcuts.yml
   @@ -0,0 +1,282 @@
   +    cache_metadata:
   +      contexts:
   +        - 'languages:language_content'
   +        - 'languages:language_interface'
   +        - url
   +        - user
   +        - user.permissions
   

   Why is it cacheable per URL? Which plugin/handler of this view causes that?

2. +++ b/core/modules/shortcut/config/optional/views.view.shortcuts.yml
   @@ -0,0 +1,282 @@
   +      cacheable: false
   

   And which handler/plugin causes the view to be uncacheable?

3. +++ b/core/modules/shortcut/src/Plugin/views/argument_default/CurrentShortcutSet.php
   @@ -0,0 +1,46 @@
   +  public function getCacheContexts() {
   +    return ['user'];
   +  }
   

   We should add a @todo here: it doesn't need to be cached per user, but per-current-shortcut-set. I.e. a new user.shortcut_set cache context.

   Can be follow-up of course. But right now, this unnecessarily reduces cacheability. This would point anybody interested user towards the issue where that will be improved.

Finally: this will conflict with #2464427: Replace CacheablePluginInterface with CacheableDependencyInterface.

1. Impossible; the cache contexts stored in the View config entity are for plugins/handlers only. Not for something that is rendered.
2. You can figure this out by saving the view while a breakpoint is set in ::calculcateCacheMetadata(). Also for point 1.
3. Well, we can fix it here, but I didn't want to hold this issue up on that. It's a pre-existing problem technically speaking, so it's technically out of scope here.

Filed #2514784: \Drupal\views\Plugin\views\argument\ArgumentPluginBase should allow subplugins to specify a more specific url cache context to fix #35.1.

@jibran: awesome
@olli: THANK YOU!

Can we split the BTB improvements into their own issue?

+1 to split

Re-roll

Unblocked now. Let's see.

This is 8.1.x material now.

Code looks pretty good, mostly only have minor comments. Will take this for a spin now. Also we need tests for the upgrade path.

1. +++ b/core/modules/shortcut/config/optional/views.view.shortcuts.yml
   @@ -0,0 +1,284 @@
   +          default_argument_type: current_shortcut_set
   +          default_argument_options: {  }
   
   +++ b/core/modules/shortcut/config/schema/shortcut.views.schema.yml
   @@ -0,0 +1,8 @@
   +views.argument_default.current_shortcut_set:
   +  type: sequence
   +  label: 'Shortcut set assigned to current user'
   +  sequence:
   +    type: string
   +    label: 'Shortcut set'
   

   This is not correct. There are actually no settings saved in the default argument handler, so the schema should not pretend there are. Let's do type: mapping and then mapping: { }

   Edit: I just looked at what other default argument plugins are doing and they are doing the same thing. So I guess we can leave it here and just update all of them together in a separate issue.

   Also found #2623548: Remove obsolete views.argument_default.php config schema :-P

2. +++ b/core/modules/shortcut/shortcut.install
   @@ -67,3 +70,57 @@ function shortcut_uninstall() {
   + * Replace existing shortcuts block with views shortcuts block.
   

   "views" -> "Views"

3. +++ b/core/modules/shortcut/shortcut.install
   @@ -67,3 +70,57 @@ function shortcut_uninstall() {
   +
   

   Remove this empty line.

4. +++ b/core/modules/shortcut/shortcut.install
   @@ -67,3 +70,57 @@ function shortcut_uninstall() {
   +  // Skip the update if views and block module are not installed.
   

   "views and block" -> "Views and Block"

5. +++ b/core/modules/shortcut/shortcut.install
   @@ -67,3 +70,57 @@ function shortcut_uninstall() {
   +  $optional_install_path = drupal_get_path('module', 'shortcut') . '/' . InstallStorage::CONFIG_OPTIONAL_DIRECTORY;
   

   Minor but you can use $module_handler->getModule('shortcut')->getPath();

6. +++ b/core/modules/shortcut/shortcut.install
   @@ -67,3 +70,57 @@ function shortcut_uninstall() {
   +    // Only update if shortcuts block exist.
   

   "Only update shortcut blocks."

7. +++ b/core/modules/shortcut/shortcut.install
   @@ -67,3 +70,57 @@ function shortcut_uninstall() {
   +      $block->delete();
   +      \Drupal::entityManager()
   +        ->getStorage('block')
   +        ->create($values)
   +        ->save();
   

   Is there a reason we are deleting and recreating the block instead of just updating it directly?

8. +++ b/core/modules/shortcut/shortcut.install
   @@ -67,3 +70,57 @@ function shortcut_uninstall() {
   +    $message = \Drupal::translation()->translate('Replaced following shortcuts block with views shortcuts block: @ids', ['@ids' => implode(', ', $ids)]);
   

   shortcut block*s* and *V*iews shortcuts block*s*.

9. +++ /dev/null
   @@ -1,41 +0,0 @@
   -class ShortcutsBlock extends BlockBase {
   

   Let's leave this as BC, now that 8.0.0 is out and just add a @deprecated

10. +++ b/core/modules/shortcut/src/Plugin/views/argument_default/CurrentShortcutSet.php
    @@ -0,0 +1,55 @@
    +    // @todo it needs to be cached per user.shortcut_set cache context.
    

    Let's remove that @todo. hook_default_shortcut_set() does not support cache metadata currently, so being able to *properly* cache shortcut (sets) is a whole can of worms, but making it per-user is a pretty good best effort IMO, so it should be fine for now.

11. +++ b/core/modules/shortcut/tests/src/Functional/ShortcutBlockTest.php
    @@ -0,0 +1,55 @@
    +    // Creates a block instance and place in a region through api.
    

    "place *it* in a region"

    The "through api" can be removed IMHO.

12. +++ b/core/modules/shortcut/tests/src/Functional/ShortcutBlockTest.php
    @@ -0,0 +1,55 @@
    +      'title' => $this->randomString(),
    ...
    +    $this->getSession()->getPage()->find('css', '#block-' . $block->id());
    +    $this->getSession()->getPage()->find('css', 'a[href="/admin/structure/block"]');
    

    Maybe also assert that the shortcut title appears.

Opened #2623568: Config schema of argument_default plugins is incorrect

Here is a comparison of the markup and some screenshots to prove that the output is the same. The added markup is just in the nature of the game of using Views, I'm not sure we can do a lot about this.

Current:

Views:

Current markup:

    <div id="block-shortcuts" class="contextual-region block block-shortcut block-shortcuts" role="navigation">
  
      <h2>Shortcuts</h2>
    <div role="form" class="contextual" data-contextual-id="block:block=shortcuts:langcode=en"><button aria-pressed="false" class="trigger focusable visually-hidden" type="button">Open Shortcuts configuration options</button><ul class="contextual-links" hidden=""><li class="block-configure"><a href="/admin/structure/block/manage/shortcuts?destination=user/1">Configure block</a></li></ul></div>
      <div class="content">
      <ul class="toolbar-menu"><li class="_"><a href="/node/add">Add content</a></li><li class="_"><a href="/admin/content">All content</a></li></ul>
    </div>
  </div>

Views markup:

    <div class="views-element-container contextual-region block block-shortcut block-views-blockshortcuts-shortcut-block" id="block-shortcuts" role="navigation">
  
      <h2>Shortcuts</h2>
    <div role="form" class="contextual" data-contextual-id="block:block=shortcuts:langcode=en|entity.view.edit_form:view=shortcuts:location=block&amp;name=shortcuts&amp;display_id=shortcut_block&amp;langcode=en"><button aria-pressed="false" class="trigger focusable visually-hidden" type="button">Open Shortcuts configuration options</button><ul class="contextual-links" hidden=""><li class="block-configure"><a href="/admin/structure/block/manage/shortcuts?destination=node">Configure block</a></li><li class="entityviewedit-form"><a href="/admin/structure/views/view/shortcuts/edit/shortcut_block?destination=node">Edit view</a></li></ul></div>
      <div class="content">
      <div><div class="contextual-region view view-shortcuts view-id-shortcuts view-display-id-shortcut_block js-view-dom-id-cf669f0007ee755b6d637bbcefe3cd19c0a456a3e34d2836f3e43976f31c8431">
  
    <div role="form" class="contextual" data-contextual-id="entity.view.edit_form:view=shortcuts:location=block&amp;name=shortcuts&amp;display_id=shortcut_block&amp;langcode=en"><button style="display: block;" aria-pressed="false" class="trigger focusable visually-hidden" type="button">Open  configuration options</button><ul class="contextual-links" hidden=""><li class="entityviewedit-form"><a href="/admin/structure/views/view/shortcuts/edit/shortcut_block?destination=node">Edit view</a></li></ul></div>
      
      <div class="view-content">
      <div class="item-list">
  
  <ul>

          <li><div class="views-field views-field-title"><span class="field-content"><a href="/node/add">Add content</a></span></div></li>
          <li><div class="views-field views-field-title"><span class="field-content"><a href="/admin/content">All content</a></span></div></li>
    
  </ul>

</div>

    </div>
  
          </div>
</div>

    </div>
  </div>

nice to see this moving again Jibran! :)

Looks great to me. Thanks a lot @jibran, really great work!

I still think it is unnecessary to remove the block class, as there is no real benefit to breaking BC for that, but I'll let committers decide that. It's not a big issue to remove that part from the patch.

+++ b/core/modules/shortcut/shortcut.install
@@ -67,3 +70,51 @@ function shortcut_uninstall() {
+  $module_handler = \Drupal::moduleHandler();
+  // Skip the update if Views and Block module are not installed.
+  if (!$module_handler->moduleExists('views') || !$module_handler->moduleExists('block')) {
+    return;
+  }

+++ /dev/null
@@ -1,41 +0,0 @@
-<?php
-
-/**
- * @file
- * Contains \Drupal\shortcut\Plugin\Block\ShortcutsBlock.
- */

So if a site is using shortcuts but not views then they'll just lose the block? Also what happens if they've placed the block - I think that now the block would be broken...

I think we'll either need to maintain the shortcut block as a fallback - but that is going to be very confusing since not sure how a fallback with blocks would actually work. I think we should just proceed with the shortcut views integration that this patch adds.

So I think it makes sense to not "upgrade" the shortcut block but provide views integration. I'm not sure of the point of providing the disabled default view? Also all the block creatiion trait stuff should be in a separate patch.

Hello,

Adding patch for Drupal 9.4.
Please review and provide the feedback.

Thanks
Siva

Fixing PHPCS for the files.

Fixing PHPCS error.

It was suggested in #72 to remove the upgrade path. That was agreed to by alexpott in the next comment so I am removing the upgrade tag,

Hello,

Created shortcuts view module.
https://www.drupal.org/project/shortcut_view

Thanks
Siva

The Shortcut Module was approved for removal in #3476880: [Policy] Move Shortcut module to contrib.

This is Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

The deprecation work is in #3569117: [meta] Tasks to deprecate the Shortcut module and the removal work in #3569121: [meta] Tasks to remove the Shortcut module.

Shortcut will be moved to a contributed project before Drupal 12.0.0 is released.