Support #attached settings in ViewAjaxController

Merge request created + updated tests. Rather than create a new method in ViewAjaxControllerTest, I opted to add to ::testAjaxView() to keep it faster as I would need to repeat a chunk of what is in that, but happy to move this to a separate method if that is the preference.

Thanks for including tests from the start

1) Drupal\Tests\views\Unit\Controller\ViewAjaxControllerTest::testAjaxView
Undefined array key 2
/builds/issue/drupal-3441920/core/modules/views/tests/src/Unit/Controller/ViewAjaxControllerTest.php:216
/builds/issue/drupal-3441920/vendor/phpunit/phpunit/src/Framework/TestResult.php:729
/builds/issue/drupal-3441920/vendor/phpunit/phpunit/src/Framework/TestSuite.php:685
/builds/issue/drupal-3441920/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:651
/builds/issue/drupal-3441920/vendor/phpunit/phpunit/src/TextUI/Command.php:146
/builds/issue/drupal-3441920/vendor/phpunit/phpunit/src/TextUI/Command.php:99
ERRORS!
Tests: 10, Assertions: 57, Errors: 1.

Issue summary appears complete.

Extra condition appears fine to me.

"Views Ajax Controller" This should be the name of the class, ViewAjaxController as well as describe the fix, not the problem.

Thanks, updated the title.

Title seems fine.

I think there might be a more generic way to fix this - left a long-ish comment on the MR.

Thanks for the tip!

The code change itself should work in reality, but the test as is is definitely going to fail. I would appreciate if you have advice on this one:

1. UnitTest: Mocking `\Drupal\core\Ajax\AjaxResponseAttachmentsProcessor` seems like a big rabbit hole, each time I think I've mocked everything, there is another service called somewhere in the pipeline that further needs to be mocked.
2. KernalTest: If I switch to a KernalTest it would be easier, but a lot of the heavy lifting this ViewAjaxControllerTest would be repeated
3. FunctionalJavascript: Or I can go for a FunctionalJavascript test and test it fully without any mocking and actually check that the response contains the correct thing (though I will probably need to add a test module to add #attached into a Handler).

Replied on the MR since I am hopeful the test complexity stems from a possibly unnecessary line.

Thanks! Yes you are right, in a real site it works with just setAttachments. So probably I need to do some tweaking with the mocking to make sure that AjaxResponseAttachmentsProcessor is actually running as well. Will work on the test a bit more.

The test could maybe just call ::getAttachments() on the response, to make sure the attachments are set?

Running the commands ought to be handled by AJAX system tests themselves.

Right of course! Much simpler, and of course AjaxResponseSubscriber is tested elsewhere already so that does not need to be covered here. Tests passing now in ddev. Thank you very much for your help here.

Okay tests passing now, thanks!

To note, I did consider adding an attached libraries as well to the ::setupValidMocks() method, but I tested with my initial commit that only covered SettingsCommand rather than attachments and the tests pass anyways for the library, so I do not think it adds anything (whereas as is, test only fails as expected and with the change test passes).

Looks great. Ran the test only job too just to see the failures.

I was worried having drupalSetting added to the preview could break views ui somehow. Haven't found an explicit way it could yet.

Not really my area of the code so leaving the commit to someone else. RTBC +1

Committed and pushed 3e486f783a to 11.x and 4d0abee5f2 to 11.0.x and af7f5ed2e1 to 10.4.x and 12097985d8 to 10.3.x. Thanks!

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