Problem/Motivation

Create a view with one required and one optional argument, enable pager and ajax. Try to click on next page. Nothing will happen, network tab for the request should display a fatal error.

Proposed resolution

Two problems. The fatal error is because $preview is not checked for being empty, so we should make that conditional to avoid the fatal errors.

Second, we need to somehow filter out the empty arguments and replace them with NULL, then views argument handling is happy again.

Remaining tasks

Write tests.

User interface changes

API changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Status: Active » Needs review
FileSize
1.44 KB

Here is a patch

jibran’s picture

Can we add some tests here?

dawehner’s picture

Even we do have a pure unit test for that class already, we should also have a proper integration test here as well.

Berdir’s picture

Issue tags: +Needs tests

I tried to extend the unit tests, but that is apparently not possible because PHPUnit_Framework_MockObject_Matcher_Parameters hardcodes an isEqual constraint, which doesn't care about NULL/empty string.

The mocking of drupalRender() somehow also prevents the recoverable fatal errors that you get when you pass NULL/FALSE to the real method.

Berdir’s picture

Ok, figured it out. Also noticed that my fix was bogus, not sure how that worked for me ;)

Not sure about integration tests then, is there already something we can extend for ajax pagers?

The last submitted patch, 5: views-ajax-args-2358603-5-test-only.patch, failed testing.

dawehner’s picture

+++ b/core/modules/views/src/Controller/ViewAjaxController.php
@@ -79,6 +79,12 @@ public function ajaxView(Request $request) {
+
+      // Replace empty argument strings with NULL.
+      $args = array_map(function ($arg) {
+        return ($arg == '' ? NULL : $arg);
+      }, $args);
+

Note: it would be kinda explain why we do things here, not what.

Berdir’s picture

Better?

Anything else? :)

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Alright, thank you sir!

  • catch committed 5738f39 on 8.0.x
    Issue #2358603 by Berdir: ViewsAjaxController results in fatal error for...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

Status: Fixed » Closed (fixed)

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