Views Block Display skips preBlockBuild() call on ajax rebuild

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.

The last submitted patch, 95: 2605218-95.patch, failed testing. View results →

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

The last submitted patch, 102: 2605218-102.patch, failed testing. View results →

Fixed test case.

The last submitted patch, 104: 2605218-104.patch, failed testing. View results →

The tests were broken by

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()

The last submitted patch, 112: 2605218-112.patch, failed testing. View results →

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

Third time's the charm?

The last submitted patch, 115: 2605218-115.patch, failed testing. View results →

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 with a 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.

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.