Admin theme not active when using batch export method and a display path starting with admin/

Uploading example views.view.content.yml file with a json export attached to the default content view which can be used for testing.

Another option would be to add an additional option such as in https://www.drupal.org/project/drupal/issues/2719797 ✨ New option for Views page displays to use the admin theme Fixed / https://git.drupalcode.org/project/drupal/-/commit/4dbf3f46c19766a38c3e4... to mark the path as an admin route directly in views.

I think instance, if the path is /admin it is showing "Yes (admin path)" - so perhaps instead of changing the formats on the route, the plugin can just mark the route as admin directly if the path starts with admin?

Patch going with the option of marking the route as an admin route.

Setting as needs review to get feedback on the approach - we should be able to add a unit test to test the route collection.

Went for a kernel test so that the test can still be valid if the implementation differs from the proposed approach.

Interdiff is the test only patch.

The last submitted patch, 7: views-data-export-3368855-7-mark-as-admin-route-test-only.patch, failed testing. View results →

Thanks Eric!

Looks good to me - approach is sensible, and fixes our unexpected behaviour with front theme on admin path.

Thanks for the patch. It fixes the problem.

+1 to get this commited

Actually the "batch" path should always use the admin theme regardless of views path starts with "/admin" or not, because this is also the case in the core:

system.batch_page.json:
  path: '/batch'
  defaults:
    _controller: '\Drupal\system\Controller\BatchController::batchPage'
  requirements:
    _access: 'TRUE'
    _format: 'json'
  options:
    _admin_route: TRUE

So here the batch uses always the admin theme and this should be the case with the patch for this issue. I removed the

str_starts_with($this->getOption('path') ?? '', 'admin/')

part from the patch.

The last submitted patch, 12: views-data-export-3368855-12-mark-as-admin-route.patch, failed testing. View results →

Actually the "batch" path should always use the admin theme regardless of views path starts with "/admin" or not, because this is also the case in the core:

This is not true and is not the case in core.

The batch negotiator (theme.negotiator.system.batch) runs at a higher priority that the admin theme negotiator (theme.negotiator.admin_theme)

See https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/syste...

@ericgsmith

Indeed, you are right. Thanks! I hid my patch.

this patch works for me.

Can you please explain your patch @erom?

If you are making changes to a patch you should supply interdiff and comment on the changes or its confusing to follow.

I've only glanced at it but it looks like the patch in 7 but without the test?

@ericgsmith, my patch will just automatically use the admin theme when it's using the batch. it is not logically correct since some use cases should allow batch to appear within a theme, i will hide my patch..

I've tested patch #17 on Drupal 10.2.6 with Views Data Export 8.x-1.4 and it works really nice, thanks @erom.

Moved patch #7 to MR

This was previously RTBC'd in #9 and #10 and #11 but subsequent patches were added which moved it back to needs work / review.

I'm incline to argue the subsequent changes to the patch have not been needed and the tests were lost - I'm leaving this as needs review.

After module update to 1.5 patch #7 can't be applied anymore. Adding re-rolled patch while MR is in review

#23 works as expected for us on 1.5.0

Patch #23 works well with no issues.

Looking at this now, we already provide the route and change some options on it, so we don't need to alter our own route I don't think.

@ericgsmith thanks so much for the issue, code and the tests, super helpful.

I've basically committed what you had, but in our method that generates the original route, rather than altering the collection of them. Seems to work fine, and your tests still pass :)

Thanks everyone!

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