Updated: Comment #0

Problem/Motivation

  • Following #2062439: Provide listing of custom block entities and #2058321: Move the 'place block' UI into the block listing, there are two entry points for creating a new custom block: The custom block listing at admin/structure/custom-blocks, and the block placement form at admin/structure/block.
  • When you add a custom block from admin/structure/block, submitting the custom block creation form takes you directly into the "place block" form for an instance of this new block, and then subsequently redidirects back to the place block form.
  • When you add a custom block from admin/structure/custom-blocks, the behavior changes based on whether you have only one custom block type:
    • If you have only one custom block type, you are redirected back to admin/structure/custom-blocks, as you'd expect.
    • If you have multiple custom block types, you first select your block type, then fill out the custom block form... and then are unexpectedly redirected into the place block workflow and then to admin/structure/block, despite that you might not have any intention of placing the block in the first place.

Video illustrating this adventure:
https://www.dropbox.com/s/b15zhahytrww9sn/custom_block_redirect.mp4

Proposed resolution

Always redirect back to admin/structure/block/custom-blocks if that's where you start.

Additionally, if you started on a theme-specific page like admin/structure/block/list/seven, you will be redirected there.

Remaining tasks

Remove the seven theme override of the custom block list theming (ie this works in stark)

User interface changes

The user is consistently redirected to admin/structure/custom-blocks if that's the starting point.

API changes

Probably none.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

larowlan’s picture

Status: Active » Closed (duplicate)
tim.plunkett’s picture

Status: Closed (duplicate) » Active

Apparently it wasn't, according to xjm.

tim.plunkett’s picture

Component: configuration system » custom_block.module
Issue tags: +Needs tests
larowlan’s picture

Grr you can see the destination in the URL on the list screen but either it's not in the links or overlay stuffs it. Hoping it's the latter cause the tests do exactly what the video does. The only difference is overlay.

larowlan’s picture

Nope doesn't work. Has to be seven.

larowlan’s picture

tim.plunkett’s picture

I'm not sure which is stranger, seven_custom_block_add_list completely bypassing the preprocess, or custom-block-add-list.html.twig using the "node-type-list" class :)

Good find!

larowlan’s picture

Status: Active » Needs review
FileSize
965 bytes
larowlan’s picture

So in terms of 'needs tests' do we need to duplicate the tests with the seven theme enabled </groan>

tim.plunkett’s picture

Can't we just gut/remove the seven override? Also, put the test into a helper method and run it twice...

andymartha’s picture

Status: Needs review » Reviewed & tested by the community

I can't comment on tim.plunkett's idea in #10, but after applying the patch in #8 by larowlan, I can confirm that desired redirect effect happens.
Here is a video of after the patch: http://www.youtube.com/watch?v=8PaGHF110Pg

tim.plunkett’s picture

Status: Reviewed & tested by the community » Needs work

This still needs tests.

larowlan’s picture

Agreed approach: remove the override in seven

tim.plunkett’s picture

Assigned: Unassigned » tim.plunkett

Working on this.

tim.plunkett’s picture

Status: Needs work » Needs review
Issue tags: -Usability, -Needs tests
FileSize
4.23 KB

Well this is fun!

CustomBlockInterface::getTheme() and CustomBlockInterface::setTheme() are never actually used.
Or, that is, setTheme() is never used.

So this isn't even really related to the OP, but is covered under the title.

Status: Needs review » Needs work
Issue tags: +Usability, +Needs tests

The last submitted patch, block-2068053-15.patch, failed testing.

tim.plunkett’s picture

Current URL is 'http://ns4007744.ip-192-95-30.net/checkout/admin/structure/block/add/custom_block%3A35922bf3-3d37-4bed-8502-2a192c94b332/seven'.
Current URL is 'http://ns4007744.ip-192-95-30.net/checkout/admin/structure/block/add/custom_block%3Ae570ce8e-e2b9-4227-9105-5d711f7568ce/bartik'.

Those are wrong because it always 'stark' due to getTheme() always returning NULL because setTheme() is never called:

CustomBlockFormController:

        if ($theme = $block->getTheme()) {
          $form_state['redirect'] = 'admin/structure/block/add/custom_block:' . $block->uuid->value . '/' . $theme;
        }   
        else {
          $form_state['redirect'] = 'admin/structure/block/add/custom_block:' . $block->uuid->value . '/' . \Drupal::config('system.theme')-          >get('default');
        }
+++ b/core/modules/block/custom_block/lib/Drupal/custom_block/Controller/CustomBlockController.php
@@ -106,6 +106,7 @@ public function addForm(CustomBlockTypeInterface $custom_block_type, Request $re
     if (($theme = $request->attributes->get('theme')) && in_array($theme, array_keys(list_themes()))) {
+      throw new \Exception('Betcha this code will never fire!');
       // We have navigated to this page from the block library and will keep track
       // of the theme for redirecting the user to the configuration page for the
       // newly created block in the given theme.
       $block->setTheme($theme);
     }
tim.plunkett’s picture

If we try to do something like block/add?theme=bartik, that breaks drupal_get_desintation (which is insane, btw).

So I don't have any obvious solutions on how to track the theme name from page to page...

larowlan’s picture

We discussed this in irc and decided not to use drupal_get_destination

tim.plunkett’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
10.61 KB
5.34 KB
10.68 KB

Okay, here we go!
We can't use a proper local action because we need a query string, so we fake it.
CustomBlockController::addForm() now checks $request->query
seven_custom_block_add_list() is not outright removed yet, but it is brought into line with template_preprocess_custom_block_add_list().

tim.plunkett’s picture

+++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php
@@ -2377,7 +2377,11 @@ protected function assertUrl($path, array $options = array(), $message = '', $gr
+    // Paths in query strings can be encoded or decoded with no functional
+    // difference, decode them for comparison purposes.
+    $actual_url = urldecode($this->getUrl());
+    $expected_url = urldecode($this->container->get('url_generator')->generateFromPath($path, $options));
+    return $this->assertEqual($actual_url, $expected_url, $message, $group);

By the way, I have this hunk in about 3 different issues, I've been doing a lot with query strings lately :) It's in #2083877: Add a theme selector to the place block form which is RTBC

tim.plunkett’s picture

That went in.

larowlan’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @timplunkett for resolving this

tim.plunkett’s picture

tim.plunkett’s picture

Issue summary: View changes

Updated issue summary.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

tim.plunkett’s picture

Assigned: tim.plunkett » Unassigned

Thank you!

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Updated

DarkteK’s picture

Component: custom_block.module » ajax system

If anyone is having this issue in Drupal 9, you can do this:

use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Url;

/**
 * Implements hook_form_alter().
 *
 * Redirects any 'Add custom block' event here: /admin/structure/block/block-content.
 */
function module_name_form_alter(&$form, FormStateInterface $form_state, $form_id) {
  $custom_blocks = [
    'block_content_xxxx_form',
    'block_content_xxx_1_content_form',
  ];
  if (in_array($form_id, $custom_blocks)) {
    $form['actions']['submit']['#submit'][] = 'module_name_block_list_form_submit';
  }
}

function module_name_block_list_form_submit($form, FormStateInterface $form_state) {
  $url = Url::fromRoute('entity.block_content.collection');
  $form_state->setRedirectUrl($url);
}
smulvih2’s picture

I still get this inconsistent behavior in 9.5.5. Adding a submit handler like in comment #28 worked for me.