Filename not applied when downloading cached export.

+++ b/src/Plugin/views/display/DataExport.php
@@ -49,6 +50,12 @@ class DataExport extends RestExport {
+      $response->headers->set('Content-Disposition', 'attachment; filename="' . \Drupal::token()->replace($filename) . '"');

Why not use $this->globalTokenReplace() here?

+1 for this patch, worked for me.

I agree with @Zemelia.
Tested on my current project with this issue, patch from #2 solved problem.

Found problem. Try to use tokens in files name and you will get this error:

LogicException: The controller result claims to be providing relevant cache metadata, but leaked metadata was detected. Please ensure you are not rendering content too early. Returned object class: Drupal\Core\Cache\CacheableResponse. in Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (line 154 of /var/www/drupalvm/drupal/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php).

Fixed in new patch version.

Nice one @hamrant, #8 works well.

Would be nice to have a to to verify this.

A test here would be really great since it deals with caching, which is always tricky.

A few questions/comments:

1. +++ b/src/Plugin/views/display/DataExport.php
   @@ -49,6 +51,13 @@ class DataExport extends RestExport {
   +    $view = Views::getView($view_id);
   

   Doesn't this object already have $this->view set?

2. +++ b/src/Plugin/views/display/DataExport.php
   @@ -49,6 +51,13 @@ class DataExport extends RestExport {
   +      $response->headers->set('Content-Disposition', 'attachment; filename="' . \Drupal::token()->replace($filename, ['view' => $view], [], $bubbleable_metadata) . '"');
   

   Is there a reason to not use $this->globalTokenReplace() instead?

Just spent the last hour or so running into this bug as well, I only had this issue on production, and not on local, but that was because I had $settings['cache']['bins']['render'] = 'cache.backend.null'; in my settings.local.php.

I don't have the time to set up a local env that allows me to run tests now, but I think this should be enough to test this:

  /**
   * Tests that the content-disposition header is set when responses cached.
   *
   * @see https://www.drupal.org/project/views_data_export/issues/2887562
   */
  public function testCachedResult() {
    $this->drupalGet('test/data_export/entity');
    $this->assertSession()->statusCodeEquals(200);
    $this->assertEquals($this->drupalGetHeader('content-disposition'), 'attachment; filename="foo.csv"');

    $this->drupalGet('test/data_export/entity');
    $this->assertSession()->statusCodeEquals(200);
    $this->assertEquals($this->drupalGetHeader('content-disposition'), 'attachment; filename="foo.csv"');
  }

I have tested the patch in #8 and it does seem to work, so great work on the solution.

I set up an env for tests, but I can't get them to work as I think they should. This test is green even without the patch applied. If someone else would like to carry on here, fixing #10 and trying to get the test to work would be great start.

I'm not sure that we should add the test here though, there is only one other functional test and it is really really basic.
Because there are so few examples asking for contributors to write tests isn't great.

Re #10.1 - unfortunately this is a static method so we're not in object context.
Re #10.2 - Same answer, it's a static method so $this isn't available.

Re #12

Because there are so few examples asking for contributors to write tests isn't great.

I agree here, we should probably open up issues to get tests in shape for this module.

+++ b/src/Plugin/views/display/DataExport.php
@@ -219,3 +204,4 @@ class DataExport extends RestExport {
+

Nit: Extra new line

Committed #8. Thanks all! (Extra line fixed on commit.)