Views pager is using exposed_raw_input instead of exposed_input

Using exposed_raw_input really breaks pager links for Date and Datetime fields.

#4 for 8.3.x cleanly applies to 8.6.2 and works fine for me.

This fixes the issue for me as well. Without the patch, views with data filters in them resulted in the pager rendering an empty result set.

In order to commit a bug fix we need an automated to test to prove that we've fixed the bug and ensure that we don't break it again in the future. For more information about writing tests in Drupal 8 see the following links:

1. https://www.drupal.org/docs/8/testing
2. https://api.drupal.org/api/drupal/core%21core.api.php/group/testing/8.7.x

Also there's quite a lot of use of $view->exposed_raw_input around - when is it right to use it? Changing this in the base class feels like it could have quite wide impacts.

The docs on ViewExecutable don't really help...

  /**
   * An array of input values from exposed forms.
   *
   * @var array
   */
  protected $exposed_input = [];

  /**
   * Exposed widget input directly from the $form_state->getValues().
   *
   * @var array
   */
  public $exposed_raw_input = [];

  /**
   * Figures out what the exposed input for this view is.
   *
   * They will be taken from \Drupal::request()->query or from
   * something previously set on the view.
   *
   * @return string[]
   *   An array containing the exposed input values keyed by the filter and sort
   *   name.
   *
   * @see self::setExposedInput()
   */
  public function getExposedInput() {

Just did run into this as well.

I also found #1802666: Try to understand the difference between the different exposed input variables., which indicates that not even @dawehner knows what all those different properties are for :) The fun part seems to be that exposed_input actually seems to be more "raw" than exposed_raw_input, which contains processed form values, so things like date objects, while exposed_input is the actual "raw" request data: $this->exposed_input = \Drupal::request()->query->all() (which AFAIK again relies on some trickery in \Drupal\views\Controller\ViewAjaxController to be populated in POST requests)

#2865401: Views pager is using exposed_raw_input instead of exposed_input is also a duplicate of this.

I had a look at writing a test for this, but the patch to use date form elements is still not committed and without that, you can't really break this, at least not with date filters that are in core. So we have a bit of a chicken/egg problem with #2648950: [PP-2] Use form element of type date instead textfield when selecting a date in an exposed filter, that would introduce this bug, but we can't commit the fix without test coverage that requires that other issue :)

I also had a look at reproducing it with an entity_autocomplete form element that has entity objects as the value. But in manual testing, I wasn't successful, the raw value is the ID, with \Drupal\taxonomy\Plugin\views\filter\TaxonomyIndexTid at least. There's a *lot* of processing/validating input going on there, possibly exactly to work around this behavior.

For reference, this is the test that I created using the current date exposed filter, but that obviously works fine as long as it is only a simple textfield.

Ran into this as well with an exposed date field. Also using the patch from #2648950: [PP-2] Use form element of type date instead textfield when selecting a date in an exposed filter.

Re-rolling patch against 8.9.x-dev , kindly review a patch.

@Hardik_Patel_12 The patch only includes the tests, it's missing the actual fix. Berdir's was a "test-only" patch :)

Rerolled the patch with both fix and the test. Please review. Thanks.

Needs re-roll

Rerolled patch in #21, thanks!

Patch to fix the tests failing in #25:
# Avoid using Drupal\Tests\UiHelperTrait::drupalPostForm(), use drupalGet() and submitForm() instead.
# Remove an empty URL parameter that is not used anymore.

It worked for us. Thanks a mil.

Rerolled for 9.5 and later (hopefully), updating the tests to work again, looks like classes and also timezone handing changed.

Tests still pass without the fix, see #13/#14, but one option would be to get this committed, then we have test coverage once that issue lands?

+1 for just adding the coverage here to avoid the chicken/egg problem. The test might not prove the bug, but it does give more confidence that we aren't breaking anything.

+++ b/core/modules/views/tests/src/FunctionalJavascript/PaginationAJAXTest.php
@@ -99,7 +99,7 @@ public function testBasicPagination() {
     // Test that no unwanted parameters are added to the URL.
-    $this->assertEquals('?status=All&type=All&langcode=All&items_per_page=5&order=changed&sort=asc&title=&page=2', $link->getAttribute('href'));
+    $this->assertEquals('?status=All&type=All&langcode=All&items_per_page=5&order=changed&sort=asc&page=2', $link->getAttribute('href'));

Sorta gave me pause, since this removes more unwanted parameters, so how come this wasn't removed before? Since it's empty, it looks ok, but still an unexpected side effect I think?

Hi I have created a MR . Please Merge . Thanks

@kunal_sahu, I am removing credit per How is credit granted for Drupal core issues.

> Sorta gave me pause, since this removes more unwanted parameters, so how come this wasn't removed before? Since it's empty, it looks ok, but still an unexpected side effect I think?

Yeah, looks like the different kinds of exposed input things handle empty parameters differently, I think that's an acceptable change and based on you setting it to RTBC, I assume you agree :)

Committed and pushed a487817dbe to 10.1.x and 9041793c01 to 10.0.x. Committed 3664a3d and pushed to 9.5.x. Thanks!

The tests are now fixed in #32, good!
For the rest, it is the same fix as before (#25), that was already reviewed by the community.
Let's move back the issue to RTBC.

@dxvargas the patch committed from #32 includes the fix as was.