"Notice: Undefined index: defaults" when visiting default media_library view

isset conditions added in both .theme and .module files.

Thanks for the issue and MR.

1. Changing to 9.4.

2. Patch applies with offsets for Drupal 9.3, 9.4, and 10.

[drupal-9.3.x-dev/9.3.x] [drupal-9.3.x-dev]$ patch -p1 < 817.diff 
patching file core/modules/media_library/media_library.module
Hunk #1 succeeded at 176 (offset -2 lines).
patching file core/themes/seven/seven.theme
Hunk #1 succeeded at 347 (offset -1 lines).

[drupal-9.4.x-dev/9.4.x] [drupal-9.4.x-dev]$ patch -p1 < 817.diff 
patching file core/modules/media_library/media_library.module
Hunk #1 succeeded at 176 (offset -2 lines).
patching file core/themes/seven/seven.theme
Hunk #1 succeeded at 347 (offset -1 lines).

[drupal-10.0.x-dev/10.0.x] [drupal-10.0.x-dev]$ patch -p1 < 817.diff 
patching file core/modules/media_library/media_library.module
Hunk #1 succeeded at 176 (offset -2 lines).
patching file core/themes/seven/seven.theme
Hunk #1 succeeded at 347 (offset -1 lines).

3. Patch is very simple change.

4. Needs screenshots of before fix and after fix.

5. Needs manual testing on 9.4.

Tested on 9.4. Reproduced the issue following the steps in the ticket (see screenshots before1-3). Cleared the database log and applied the patch. Patch applied cleanly and reproduction steps no longer cause the issue (see screenshot after, showing the still-empty database log).

This points to the fact we have an untested code. It'd be great to add automated test coverage for it. \Drupal\Tests\media_library\FunctionalJavascript\ViewsUiIntegrationTest looks a good place to add it.

Kindly review patch for branch 9.5x,,

Verified on 9.5x dev drupal version. Now unable to reproduce the issue. Please check the below screenshot.
Steps performed :
1. Enable (or have enabled) media and media_library.
2. Go to /admin/structure/views/settings and check the "Always show the default display" checkbox. Save the page.
3. Visit /admin/structure/views/view/media_library
4. View the notices
5. Applied patch
6. follow step 3

@aarti zikre, when adding screenshot please add them to the Issue Summary so it helps committers and reviewers find the latest. Not so important here as this issue only has a few comments so far but it does make a big difference on issues with many comments and sets of screenshots.

This is tagged as 'needs tests' and there are not tests in the patch. Tests were asked for in #9.

The proposed resolution section of the Issue Summary is full of questions. It should be a statement of what is being changed and why so the reviewer can determine that the intended fix was implements. Adding tag for an issue summary update.

Also seeing this issue with claro. So makes me think patching seven or theme isn't the approach.

Adding a test. and added a line to the media_library.module that solves the warning without having to edit any of the themes.

Issue summary seemed clear to me (least the testing steps) so not sure if that label could be removed.

Still needs tests, see #9

Added a tests patch but looks like the regular one failed

Took a different approach for the tests but should work.

Patch #18 resolved the issue.

@Munavijayalakshmi Thanks for testing.

This still needs code review and core gate checks.

This does not need more manual testing, thanks!

I wonder if this is a problem for all views, by default. Does Views simply not add the css_class key to the default display normally, and therefore any view would be affected by this problem, if it had a hook trying to access ['defaults']['css_class']?

Then again, Media Library is the module that's implementing hook_views_pre_view, so it should be acting more defensively. Therefore, I think this makes sense.

+++ b/core/modules/media_library/tests/src/Functional/ViewsIntegrationTest.php
@@ -0,0 +1,64 @@
+    // Configure to always show the default display.
+    $edit = [
+      'ui_show_default_display' => TRUE,
+    ];
+    $this->drupalGet('admin/structure/views/settings');
+    $this->submitForm($edit, 'Save configuration');

Actually, one question: why do we need this section, or indeed, why does this need to be a functional test? We're rendering the view programmatically in this test, so in theory, wouldn't we get this error even if we weren't interacting with the UI?

Moving to NW to convert test to a kernel test.

Converted to kernel

Assuming this passes tests, I like it.

phpunit -c app/core app/core/modules/media_library/tests/src/Kernel/ViewsIntegrationTest.php 
Bootstrapped tests in 0 seconds.
PHPUnit 9.6.6 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\media_library\Kernel\ViewsIntegrationTest
.                                                                   1 / 1 (100%)

Time: 00:01.139, Memory: 10.00 MB

OK (1 test, 1 assertion)

Unfortunately that test doesn't fail without the fix

So what is we go back to functional tests in #18 those do fail.

I agree that #18 is a good solution. Code looks great and I can't find anything that needs to change.

+++ b/core/modules/media_library/tests/src/Functional/ViewsIntegrationTest.php
@@ -0,0 +1,64 @@
+    unset($build['#view']->display_handler->options['defaults']);
+    // If this fails then there is a bug.
+    $this->assertNotNull($renderer->renderRoot($build));

Comments should help the user understand the test. For me, this is merely a statement of the obvious. As someone who does not know the render system this is meaningless.

Also, the title of this is an error message, can we change it to what is being fixed?

It is always challenging to decide to push back on such matters but these details help those who come behind us.