Views Block Display skips preBlockBuild() call on ajax rebuild

Interesting issue

Doing a bit of triage: this affects one piece of functionality and is a self-contained bug. While important, it is not major.

Marked #2973751: Views block settings are lost when using AJAX as duplicate of this one.

Fixed path.

+++ b/core/modules/views/src/Controller/ViewAjaxController.php
@@ -176,6 +176,15 @@ public function ajaxView(Request $request) {
+          $blocks = \Drupal\block\Entity\Block::loadMultiple();
+          foreach ($blocks as $key => $block) {
+            $settings = $block->get('settings');
+            if (isset($settings['items_per_page'])) {
+              $view->setItemsPerPage($settings['items_per_page']);

What if there is more than one block, with different items per page each?

I believe the hard part of this issue is figuring out the block instance to get the correct contexts, in the first place.

Then, I believe we should call ::preBlockBuild() with the correct contexts loaded, once there may be other settings (overrides) that can be defined by contrib/custom modules (not only items_per_page).

@buenos, are you still working on this issue? I've unassigned it, but please feel free to assign it back to you if that is the case.

This patch implements a solution that should be generic to all block overrides, once we call ::preBlockBuild() even if previewing the view in an AJAX request. This solution relies on a fix from #2823541-34: Table clicksort is lost when using views exposed filter & Pager exposed '#items', so the ajax post requests persist the query params. I'm attaching then a patch with this fix only, and also a combined patch so we can trigger the testbot.

Oh didn't realize the dependency patch has some failing tests... :(
But our test-only fail test passed! :)

OK, not sure if this is the best way to fix the failing tests, but it seems to fix things for me.

Again, the combined patch includes this fix and #2823541-34: Table clicksort is lost when using views exposed filter & Pager exposed '#items'

1. +++ b/core/modules/views/src/Plugin/views/display/Block.php
   @@ -64,12 +107,24 @@ class Block extends DisplayPluginBase {
        $this->blockManager = $block_manager;
   +    $this->currentRequest = $request_stack->getCurrentRequest();
   +    $this->keyValue = $key_value;
   +    $this->contextRepository = $context_repository;
   +    $this->contextHandler = $context_handler;
      }
   

   Always store the request stack in the property, never the current request. The point of the stack is that the current request can change, this breaks that.

   To be nice to subclasses, we could default the new dependencies to NULL and fall back to \Drupal::service()

2. +++ b/core/modules/views/src/Plugin/views/display/Block.php
   @@ -350,12 +409,181 @@ public function blockSubmit(ViewsBlock $block, $form, FormStateInterface $form_s
   +    // We store the instance overrides in the key/value store, to be retrieved
   +    // when an AJAX call happens and we don't have the configuration from the
   +    // context. We pass then just a hash of the key as a query parameter to
   +    // subsequent calls.
   +    $key = $this->currentRequest->request->get('instance_overrides_key');
   

   Not sure about the "instance" thing. Maybe something like block_config_key instead or so?

3. +++ b/core/modules/views/src/Plugin/views/display/Block.php
   @@ -350,12 +409,181 @@ public function blockSubmit(ViewsBlock $block, $form, FormStateInterface $form_s
   +    // We expect to receive here the "instance_overrides_key" parameter, which
   +    // will allow us to retrieve the block config from the key/value store.
   +    $query_args = $this->currentRequest->request->all();
   +    // In order to have exposed filters submissions preserve the query args as
   +    // well, they are injected in the 'view_query' param. We merge them all
   +    // together here.
   +    if (!empty($query_args['view_query'])) {
   +      $view_query = [];
   +      parse_str($query_args['view_query'], $view_query);
   

   Not quite sure where things should live.

   I was wondering if part of it could be in \Drupal\views\Controller\ViewAjaxController::ajaxView(), some parts of that are duplicated here. Then again, if we move it over there, we have to assume other parts, like check if it is a block display plugin.

4. +++ b/core/modules/views/src/Plugin/views/display/Block.php
   @@ -350,12 +409,181 @@ public function blockSubmit(ViewsBlock $block, $form, FormStateInterface $form_s
   +    // We need the dom_id of this view so future AJAX calls can rely on the
   +    // same selector. However, ViewAjaxController::ajaxView() explicitly
   +    // removes this value from the request, so we need to get it directly from
   +    // $_POST.
   +    // @todo: Maybe we shouldn't remove it in the first place?
   +    if (!empty($_POST['view_dom_id'])) {
   +      $dom_id = $_POST['view_dom_id'];
   +      $dom_id = isset($dom_id) ? preg_replace('/[^a-zA-Z0-9_-]+/', '-', $dom_id) : NULL;
   +      if (!empty($dom_id)) {
   

   Is this a left-over of the earlier approach to call $block->build() which re-created the view? This view should still have it because it is set in that very ViewAjaxController?

@Berdir thanks for reviewing!

1- Done, thanks!
2- 👍
3- Me neither. Being the cause of this bug the fact that we bypass ::preBlockBuild(), I tend to see this is more related to the block plugin, so I'd prefer to have some duplication here rather than in the main AJAX controller. But no strong opinions really.
4- yes, here the dom_id is the same, so no need for that anymore, thanks!

1. +++ b/core/modules/views/src/Plugin/views/display/Block.php
   @@ -52,6 +60,41 @@ class Block extends DisplayPluginBase {
      /**
   +   * The current request.
   +   *
   +   * @var null|\Symfony\Component\HttpFoundation\Request
   +   */
   +  protected $currentRequest;
   
   @@ -64,12 +107,24 @@ class Block extends DisplayPluginBase {
   +    $this->currentRequest = $request_stack->getCurrentRequest();
   
   @@ -350,12 +409,181 @@ public function blockSubmit(ViewsBlock $block, $form, FormStateInterface $form_s
   +    $key = $this->currentRequest->request->get('instance_overrides_key');
   

   I'm wondering whether you should use \Drupal\views\ViewExecutable::getRequest instead here. No injection would be needed

2. +++ b/core/modules/views/src/Plugin/views/display/Block.php
   @@ -350,12 +409,181 @@ public function blockSubmit(ViewsBlock $block, $form, FormStateInterface $form_s
   +    if (empty($key)) {
   +      // Calculate a brand new key.
   +      $this->instanceOverridesKey = $this->calculateConfigurationHash($config);
   +      $key_value_storage = $this->keyValue->get('views_block_overrides');
   +      if (!$key_value_storage->has($this->instanceOverridesKey)) {
   +        $key_value_storage->set($this->instanceOverridesKey, $config);
   +      }
   +    }
   

   We should document that this is triggered both in the ajax and non ajax case and explain why this is useful :)

3. +++ b/core/modules/views/src/Plugin/views/display/Block.php
   @@ -350,12 +409,181 @@ public function blockSubmit(ViewsBlock $block, $form, FormStateInterface $form_s
   +    else {
   +      // The received key didn't validate.
   +      throw new AccessDeniedHttpException('Invalid instance overrides key.');
   +    }
   

   As alternative one could set $view->build_info['fail'] = TRUE;

4. +++ b/core/modules/views/src/Plugin/views/display/Block.php
   @@ -350,12 +409,181 @@ public function blockSubmit(ViewsBlock $block, $form, FormStateInterface $form_s
   +      parse_str($query_args['view_query'], $view_query);
   

   There is a bit nicer Drupal method for that, in case you want to use it: \Drupal\Component\Utility\UrlHelper::parse

Ooops, wrong patch.. But it was actually only a re-upload of the same test-only patch from above.

@dawehner thanks for reviewing!

This should address all points from #24.

I've also realized that the patch I am incorporating from #2823541: Table clicksort is lost when using views exposed filter & Pager exposed '#items' does not fully address that issue, it's only a partial fix. So I guess it's just easier to assume those changes as part of this issue as well, and whatever issue gets in first, the other will need a re-roll.

1. +++ b/core/modules/views/src/Plugin/views/display/Block.php
   @@ -410,11 +399,17 @@ public function blockSubmit(ViewsBlock $block, $form, FormStateInterface $form_s
   +    // following calls pass along the 'block_config_key' query param and it
   +    // matches the key we are generating here for this view+display combination.
   +    // @see self::preview()
   +    // @see self::getConfigurationFromHashedKey()
   +    $key = $this->view->getRequest()->request->get('block_config_key');
   

   I think @see is only allowed in a docblock, in an inline comment, you just make it a "See .." and part of a sentence. Also not sure if self:: can actually be resolved by api.drupal.org

2. +++ b/core/modules/views/src/Plugin/views/display/Block.php
   @@ -429,8 +424,8 @@ public function preBlockBuild(ViewsBlock $block) {
        }
        else {
   -      // The received key didn't validate.
   -      throw new AccessDeniedHttpException('Invalid block config key.');
   +      // The received key didn't validate, do not build the view.
   +      $this->view->build_info['fail'] = TRUE;
        }
   

   Do we have test coverage already for this scenario? Never seen that before and wondering what that exactly does then.

   Wouldn't it need an early return then?

@Berdir thanks for the feedback!

1- Done, thanks!

2- I didn't know this existed either... I think the early return isn't necessary because it appears to abort the render process entirely: http://cgit.drupalcode.org/drupal/tree/core/modules/views/src/ViewExecut... But I guess it won't hurt either.
About test coverage, not sure the best way to check that... should we manually issue an AJAX request to the views ajax handler with a fake key, and check that the response does not contain the view markup? (any example you know of something similar I could check?)

Thanks!

1. +++ b/core/modules/views/src/Plugin/views/display/Block.php
   @@ -350,12 +398,176 @@ public function blockSubmit(ViewsBlock $block, $form, FormStateInterface $form_s
   +        $key_value_storage->set($this->blockConfigKey, $config);
   

   I'm not convinced that storing this in key/value is a good idea. At the very least we need to do some sort of manual clean up of everything we store here. Like react to View deletions/updates and block config updates. Alternatively move to key/value-expire, but then you'd have the bug return if a page persist for longer then a week. So again, not ideal.
   But otherwise this will bloat key/value and never get cleaned up, which sounds bad.

2. +++ b/core/modules/views/src/Plugin/views/display/Block.php
   @@ -350,12 +398,176 @@ public function blockSubmit(ViewsBlock $block, $form, FormStateInterface $form_s
   +    $data = serialize($configuration) . $this->view->id() . $this->view->current_display;
   

   this should have a delimiter to prevent collisions

1. I was thinking about that too. Entity Autocomplete (where we stole the whole concept from) has the same problem. I thought about using expire too and the long-cache problem. What if we just set the expiration to a very long time (like a year), then it will get deleted *at some point* if not used anymore? (Would also need a refresh logic then, like updating it if it's less than a X or so :-/)

Ran into this today, here is a reroll, unfortunate that KV storage had to get involved to persist the settings, but I couldn't see a better way of doing it when looking into the problem.

I understand the problem with a growing KV table, but it looks like a new entry is only added when the block configuration itself changes? Not ideal, but practically speaking block configuration shouldn't change so frequently that this would really become a huge problem.

I like the idea of setting a long expiry as a solution. What about setting the KV entry every time the block is rendered, so there would be little doubt it would be called again in the span of a year. A loaded page or cached response would then have to survive for longer than a year to trigger any sort of bug.

Looks like the failed test is unrelated to this issue? Queued for retesting.

Here's a reroll of the patch in 29 against Drupal 8.7 (current stable). It did not apply cleanly, so I had to manually resolve the conflict and rebase it on 8.7.x.

I'm using the patch from comment 40 against 8.7.x and ran into an issue with the layout_builder.entity context. This context causes the patch to fail in Drupal\views\Plugin\views\display\Block::getBlockFromAjaxRequest() due to the context not being in the format it expects. Filtering out the context there avoids the issue and doesn't seem to prevent the context from working.

It seems like this issue should be fixed by https://www.drupal.org/project/drupal/issues/3018782 so this tweak will likely become obsolete in a few Drupal versions. I'm uploading my patch in case anyone is in the same situation as I am, but if you're on 8.8.x and/or not using layout builder you probably want tobiasb's patch above.

@Dylan Donkersgoed

We use Layout Builder in 8.8.*. Do you mean that the patch can not work/is wrong then?

@tobiasb I wasn't able to apply patch 44 against 8.7.x so my patch is based on patch 40. If you have the same issue on 8.8.x (you might not, I haven't checked) you could add the same change to your patch. You can check by:

1. Configuring an entity bundle (e.g. a node type) to use layout builder and allow overriding the layout (not sure if it affects non-overridden layouts)
2. Configuring a view to have a block display that allows overriding the items per page and takes a contextual filter of your entity type (e.g. a reference to a node ID) and configuring the validation to take an entity of that type
3. Ensuring that a few items appear in the view (e.g. by adding some nodes that reference your initial node)
4. Creating an entity (e.g. of your node type) with an overriden layout and placing the view block in a layout taking the current entity as the context
5. Saving the node/layout
6. Testing the pager on the block

Apparently that change is still required for 8.8.x. I've attached an updated version of tobiasb's patch against 8.8.x with my change added.

Re-roll for 9.1.x

Addressing fails.

I ran into this problem when placing a Views block via Layout Builder that was not respecting the number of items set in the block config on subsequent ajax page loads. I applied the patch from #53 and that solved the problem for me here. Marking this RTBC since the tests are passing above on #53 to try and get this moving forwards again after so many years.

This has coding standard issues that need to be resolved.

Apologies if this has already been covered somewhere. I don't understand why the more recent patches abandoned the original approach of making changes to Drupal\views\Controller\ViewAjaxController::ajaxView(). There have been several comments to the effect that the Key/Value solution has a potential garbage collection issue with proliferating entries. @Sam152 indicated he didn't think that would be a big problem, but it seems to me that with Layout Builder and every block on every LB page having its own configuration that over time this would be quite a big issue. Especially when as site is first under development, there are a lot of changes to block settings going on.

Is there not a way to detect the Drupal\views\Plugin\views\display\Block instance being used and run its preBuildBlock() method? I'm probably missing something, but the current approach seems to be quite a big workaround.

After analyzing the code a bit more and doing some tinkering, I can't find any obvious way to do what I was suggesting, so not sure if that is actually possible.

The issue that I have run into and was hoping one of the patches on this thread might solve for is the fact that we need to pass some information between a block instance and the underlying view it is generating. We are attempting to do this via HOOK_preprocess_block(), but that never gets triggered when the view is refreshed via AJAX since the view sits inside the block container and the block container remains on the page. Also tried HOOK_block_build_alter and HOOK_block_view_alter and neither are triggered by the AJAX refresh of the view, even with the patched code updates.

So, I guess this is a little off topic, but if anyone has an idea about how to bridge that gap, I would be very grateful. My best idea at this point for how to proceed is to introduce an event dispatcher that runs during preBuildBlock.

I have figured out a way to reliably pass some of the settings I need to the view from the block during an AJAX request. This is accomplished by saving any extra data with the block configuration during submission of the block form.

However, the problem that I am running into now is that some view modifications are not "sticking". For example, I am trying, during preBlockBuild, to hide certain fields based on user the selection (like ctools_views):

foreach (array_keys($fields) as $field_name) {
  $this->view->removeHandler($display_id, 'field', $field_name);
  if (empty($config['fields'][$field_name]['hide'])) {
    $this->view->addHandler($display_id, 'field', $fields[$field_name]['table'], $fields[$field_name]['field'], $fields[$field_name], $field_name);
  }
}

During a non-AJAX load, the code above works just fine, thanks to the fact that my custom views block display classes' preBlockBuild method runs well in advance of ViewExecutable::preExecute(). When the view is generated via AJAX request though, it is generated using the ViewExecutable::preview() method, which runs ViewExecutable::preExecute() before passing it off to the display handler's preview method. The patch in https://www.drupal.org/project/drupal/issues/2605218#comment-13784355 allows the preBlockBuild method to run during preview, but by then it is too late for the snippet of code to do any good.

I don't have a good idea yet of how to adjust the patch to fix this issue, but wanted to get these notes down while they are fresh in my head.

The patch I'm attaching introduces a new preparePreview() method to Drupal\views\Plugin\views\display\DisplayPluginInterface, Drupal\views\Plugin\views\display\DisplayPluginBase, and adds an implementation of it to Drupal\views\Plugin\views\display\Block. It also provides execution of the new display handler method during Drupal\views\ViewExecutable::preview() prior to running the preExecute() method. I moved the logic that was added to Drupal\views\Plugin\views\display\Block::preview() in a previous patch to the new method to ensure that the preBuildBlock() method runs prior to ViewExecutable::preExecute(), which allows changes to the view based on block configuration to happen prior to all the initialization of the elements that are impacted by these changes.

I'm not sure that 'preparePreview' is the best name for the method, so if anyone has a better suggestion, I'd be happy to hear it. Any other feedback is welcome as well!

Forgot to upload the interdiff.

Just tried patch #73, with core 9.3-beta1, not working.

My view is rendering a block with a views_pre_render hook and setItemsPerPage with 3 items. With AJAX enabled, the first build is taking the value from the view parameters and not the hook.

Added reroll of patch #73 on Drupal 9.4.x.

Queuing up tests.

Setting back to "Needs work" based on #74

I had this issue and applying the patch from #73 on Drupal 9.3.14 fixed this for me.

+1
am using #73 for 2 projects at this time.

Patch #77 wasn't getting applied for D9.5 as well as D10.1.x, I have rerolled the patch for compatibility

The issue summary needs to be updated with the current solution.

Fixed custom command failed of #84 for 10.1.x.

Came across this issue after finding a similar issue happening on a client site.

The patch solves the problem itself, ajax requests respect the items per page, but the block_config_key parameter is duplicated in the URL on each pager click.

It looks like the changes to the ajax.js aren't checking if the key already exists in the URL.

Setting back to needs work for that.

Also got bit by this today.

Updating attributions

Since it doesn’t show changes what was updated?

I'm adding myself back in for credit on the commit, since versions of this since my patch use the preparePreview() method I introduced.

Hmm... I guess I don't have permission to update credit.

Re-rolled the patch from #84 against 9.5.x

Since I've been bit by this before going to try and help get it over the finish line.

Updated issue summary and updated patch for D10 practices.

Had to update the test case to do specific checks for view_query vs full array compare.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Does the bot still works? l-)

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

still works, patch does not apply anymore. Last test run was last month.

Rerolled

Fixed test case.

The tests were broken by #956186: Allow AJAX to use GET requests

I ended up removing

    // Test that no unwanted parameters are added to the URL.
    $this->assertEquals('?view_query=_wrapper_format%3Ddrupal_ajax&status=All&type=All&langcode=All&items_per_page=5&order=changed&sort=asc&wrapper_format=drupal_ajax&page=2', $link->getAttribute('href'));

Even without the branch the Hrefs are crazy long. Not sure it's in scope of this issue to fix that part.

Both points in #31 still need to be addressed.

Also, the test coverage needs to be expanded, we are making changes to javascript files so we need to have a javascript test I'd think.

I've tested patch #94 on my project running Drupal 9.5.8 and it worked as expected =)

In my case, I have a view listing articles with views_infinite_scroll to use "Load More" as pages. The load more functionality + AJAX worked fine when the views block was placed on the page using the default settings.
However, when I updated the settings to display 3 blocks per page instead of 12, the "Load more" broke and the "No results" message.

Attaching a video/screencast of my scenario.

Having a look at the patch, I think #88 still needs to be addressed too

Actually, I'm sorry about the early celebration on patch #94.

It seems that issue #88 also happens to me.

The second time I click on "Load more" the block_config_key and page are added to params in the URL. I'm using AJAX view though.

I reworked this a bit on the 10.1.x branch addressing the previous comments:

• #88: I removed the JS changes, they did not seem necessary and was creating a very long query string. Click sorting persists between AJAX requests just fine. Not sure why we needed it in the first place, but in my testing, it works fine without it.
• #31 (1) I kept the key_value implementation but I added hooks for view_update, view_delete, block_update and block_delete. Whenever we update/delete a view or block, we remove the key_value entry. Thus eliminating the risk of bloat adding up over time
• #31 (2) I changed the key_value structure to be a nested array, keyed by the view id and reworked a bit the function used to generate the hash. Thus eliminating the risk of colisions.
• The last patch was not working at all on 10.1.x because it was using ->getRequest()->request->get() I changed it to ->getRequest()->query->get()

Applied this to 11.x and fixed a failing test case.

Third time's the charm?

I think I fixed the error in the test that was failing, but I didn't really understand what I was doing.

Left some comments .

When I try to apply #115 on Drupal 10, it applied successfully. However, I get this

Fatal error: Cannot redeclare views_view_delete() (previously declared in /var/www/web/core/modules/views/views.module:809) in /var/www/web/core/modules/views/views.module on line 835

when I run drush cr

Running the tests again.

Seems some failure is happening.

Learning curve for adding the deprecation notice :)

Thanks for taking care of that. Lets see if we can move this along and hopefully get into 10.2!

I read the issue summary and comments and I didn't find any unanswered questions. I was concerned that #31 and #88 were missed but there were responded to in #112. And there are no outstanding tags here. I didn't look at the MR (it is too late).

Re-rolling the #94 for 9.5.x since I need to use it for one project.
Mostly it was minor nit pick related to coding standard.
All 3 test cases are passing on local.

I've hide the patch since 9.5.x support is going to end soon.
Keeping the status as it is.

I cam back and some of the MR. Since I found a few things that should be fixed I am setting this back to needs work.

Love this new test-only feature

There were 2 errors:
1) Drupal\Tests\views\Unit\Plugin\views\display\BlockTest::testBuildNoOverride
PHPUnit\Framework\MockObject\CannotUseOnlyMethodsException: Trying to configure method "calculateConfigurationHash" with onlyMethods(), but it does not exist in class "Drupal\views\Plugin\views\display\Block". Use addMethods() for methods that do not exist in the class
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/Framework/MockObject/MockBuilder.php:253
/builds/issue/drupal-2605218/core/modules/views/tests/src/Unit/Plugin/views/display/BlockTest.php:104
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/Framework/TestSuite.php:684
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:651
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/TextUI/Command.php:144
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/TextUI/Command.php:97
2) Drupal\Tests\views\Unit\Plugin\views\display\BlockTest::testBuildOverride
PHPUnit\Framework\MockObject\CannotUseOnlyMethodsException: Trying to configure method "calculateConfigurationHash" with onlyMethods(), but it does not exist in class "Drupal\views\Plugin\views\display\Block". Use addMethods() for methods that do not exist in the class
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/Framework/MockObject/MockBuilder.php:253
/builds/issue/drupal-2605218/core/modules/views/tests/src/Unit/Plugin/views/display/BlockTest.php:104
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/Framework/TestSuite.php:684
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:651
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/TextUI/Command.php:144
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/TextUI/Command.php:97
--
There was 1 risky test:
1) Drupal\Tests\views\Unit\Plugin\views\display\BlockTest::testBuildNoOverride
This test did not perform any assertions
/builds/issue/drupal-2605218/core/tests/Drupal/Tests/Listeners/DrupalListener.php:65
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/Framework/TestResult.php:452
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/Framework/TestResult.php:980
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/Framework/TestSuite.php:684
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:651
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/TextUI/Command.php:144
/builds/issue/drupal-2605218/vendor/phpunit/phpunit/src/TextUI/Command.php:97
ERRORS!

Remarking as appears feedback @quietone has been resolved, all threads closed, and green.

Couple of questions on the MR.

Here's a patch version of MR 4399 (latest commit 21efdb1b), if anyone else needs it.
PS: our setup enforces static patches and MR diffs are not allowed (as they can change unexpectedly).

Re-roll of patch #112 with branch 10.3.x

I hate to be the bearer of bad news on such a long sought after solution for this, but the patches above do not work for 10.3 (I can't verify for Drupal 9 and below). This is also not an ideal nor practical solution. The getBlockFromAjaxRequest() method that was added does not return the expected results. The block gets a duplicate config key and also returns null on the first ajax request via elementPreRender(). The preBlockBuild() function does not get called still because the ajax architecture only returns the view (the block is a wrapper around the view and does not get updated).

My solution for the client that needed this was as follows:
1. Add Drupal JS settings for views blocks: $build['#attached']['drupalSettings']['viewsBlock'][$this->view->dom_id] = $block->getConfiguration();
2. Add a new method to the views block display and views block plugin to add the settings to the build: alterBlockBuild(ViewsBlock $block, &$build)
3. Add an event subscriber that responds to the ViewAjaxResponse response and checks for the block display handler
4. Add an ajax command that responds on ajax and uses the block settings for updating the view

I really think our proposed solution should be changed to something similar to above. This way is much cleaner and follows the ajax workflow mostly with views. Any module that overrides views block settings could then implement their own subscriber and ajax commands with the new viewsBlock drupal settings in the block build.

I spent over 25 hours on this issue, and the only way I could get block settings to update the view, was via drupalSettings on the views block and an ajax command that fires on the response of ViewsAjaxResponse.

Here's a great working example without the viewsBlock drupal settings:
https://thinktandem.io/blog/2020/04/23/altering-views-ajax-in-drupal-8/

This is indeed an interesting issue. I just stumbled upon this issue in debugging and researching the issue I am facing.
I'm not really sure if my issue and this issue are the same issue with the same root cause and necessary solution, but it seems somewhat related, so I thought I would chime in with my case.

I have a block and I am loading a View programmatically.
The view has pagination enabled and I am seeing odd behavior on the rendered view when using the pagination.

I believe that the Ajax pagination is losing sight of how the View should be processed for rendering.

I am seeing things like:
* Duplicated results in the view
* Missing results in the view

I can sort of confirm this by enabling XDebug, and when using the Ajax pagers in the view, XDebug does not hit on the lines where I am loading and setting properties in the view for how I need it manipulated and rendered in my block.

NOTE that in 11.1.x, the line in views.module:

use Drupal\Core\Entity\EntityInterface;

was removed. So the hooks added via this issue patch cause a problem because the autoloader doesn't resolve the class properly. In the hook:

function views_block_update(EntityInterface $entity) {

when a `Drupal\block\Entity\Block` entity is passed to the hook, it gives an error:

TypeError: views_block_update(): Argument #1 ($entity) must be of type EntityInterface, Drupal\block\Entity\Block given in views_block_update() (line 482 of /var/www/docroot/core/modules/views/views.module).

This happens when you try to do a clean install of any profile that contains config for blocks.

I don't have a real chance to test this patch or update it at the moment, but at least the `use` statement needs to be re-added to the top of views.module file.

Hrm, looks like I still need this patch, so here is a re-roll for D11.1.1 that adds the `use` statement just to get past this error.

In general this patch needs more work to deal with the changes to how core is handling hooks as per #3483599 where the views.module file was refactored.

Rebased MR on latest 11.x, adjusting the hooks to be placed in classes.

Hide patches

Patch to be applied against 11.2

>= 11.3

Updated MR, snapshot applies to 11.3