Exposed filter values ignored when using batch

Not sure if I am missing something obvious. Also not sure if this is the correct approach.

That is clearly the wrong patch :) sry

@Primsi what version of Drupal core are you on? I reported 🐛 Batch exporting is broken in Drupal 10.2 Active today which sounds very similar, but it's only happening once upgrading to 10.2. On 10.1 exposed filters work fine.

That was on 10.2.0-beta1 IIRC.

We are facing the same issue once upgraded to Drupal core 10.2, the batch results are not filtered anymore.

I tried the patch in comment #3 and it fails to fix the issue for us unfortunately.

We get the same unfiltered results with and without the patch.

Looks like the issue surfaced because of this core commit: https://git.drupalcode.org/project/drupal/-/commit/66ddcd2abd3ea7e26bde7...

- if ($batch = batch_get() && isset($batch['current_set'])) {
+ if (($batch = batch_get()) && isset($batch['current_set'])) {

Previously, order of operations means the condition always fails, because && evaluates before=, so isset($batch['current_set']) is always FALSE, because $batch does not have a value yet.

With the change, if a batch is running, the form is not rendered on the page, and \Drupal\views\Form\ViewsExposedForm::submitForm does not run and set $view->exposed_data.

I tested on a vanilla Drupal 10.2.0 standard install with VDE 1.4.0, and the patch worked for me. If it does not resolve for others, maybe there's something else that \Drupal\views\Form\ViewsExposedForm::buildForm or \Drupal\views\Form\ViewsExposedForm::submitForm does that still needs to be accounted for.

Confirming reverting the commit in #10 fixes the issue for me. Moving to the core queue.

If the previous behaviour skipped this condition entirely every time (you can see in your IDE that $batch is unused), it tells me that there's no test coverage for that and it didn't do anything before? Maybe we just ditch the condition entirely? I'm not too sure how we go about writing a test for this in core, I'll do some digging for where that code came from in the first place.

That code originally came from this commit https://git.drupalcode.org/project/drupal/-/tree/a626abb24faa51ac140f737...

"Add the 7.x-3.x Views branch." - i.e when the contrib views module was merged into core (I think). Trying to track down when it was added to the contrib views.

Funnily enough, this exact same fix was done in D7 views #3119249: Undefined variable '$batch' → and that also caused issues with VBO #3253277: 7.x-3.25 broke batching in VBO → and in THAT issue, that code block was removed entirely.

Considering we clearly never had testing around this block of code and it was removed from the source where it came from so many moons ago, I reckon we just ditch it from modern Drupal too!

I should clarify that I applied the patch from #3 🐛 Exposed filter values ignored when using batch Needs work against views_data_export, and that worked for me. But I agree that since that conditional in core code did not work and did not have a test, finding out if we need it at all is best path forward.

I can confirm MR 5883 works for me.

Testing steps:
1. Install Drupal 10.2.0 with standard profile
2. Apply MR diff
3. Install Views Data Export 1.4.0
4. Create a view for Article content, with a page at /articles. Set fields to Title and body and added exposed filter on title
5. Create a data export display on the view, attached to the page. Set format to CSV, path to articles.csv, and set to use batch
6. Create some sample Article content
7. Go to /articles and filter on title
8. Click CSV icon for export and confirm CSV rows match filtered rows

This doesn't have tests, but I'm not sure how to test that something that never worked still doesn't work, so I think it can go to RTBC if others confirm.

I can confirm the merge request !5883 fixes the issue for me as well on 10.2.0!

Thanks!

We actually have a Views test that uses a batching action: \Drupal\Tests\views\Functional\UserBatchActionTest::testUserAction

Not sure if this issue can be replicated with just core, but if it can, it would be good to add a test at some point.
I agree that we don't need to add coverage in this issue, fixing the regression is more important, but I would suggest making a follow up issue to expand the coverage in that test a bit, it's very minimal as it stands.

Nearly cross-posted - I just opened the follow-up, added that test as the starting point: 📌 Add test coverage for views exposed filters and batch Active

Committed/pushed to 11.x and cherry-picked to 10.2.x, thanks!

I've added this to the known issues on https://www.drupal.org/project/drupal/releases/10.2.0 →

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