The data export supports just the two default file system schemes public:// and private://
There are many contrib modules which register additional file system schemes (for example saving to AWS S3 and Dropbox), and drupal allows configuration of the default value for this which can be set at /admin/config/media/file-system and is saved to the config item 'system.file' as 'default_scheme'.
Views Data Export doesn't recognise these additional filesystems, however, and won't allow exports to be saved to these.
Instead of choosing public or private, perhaps we could instead use the same selection mechanism for file system wrappers as used by core:
$config = $this->config('system.file');
$form['file_default_scheme'] = [
'#type' => 'radios',
'#title' => t('Default download method'),
'#default_value' => $config->get('default_scheme'),
'#options' => $options,
'#description' => t('This setting is used as the preferred download method. The use of public files is more efficient, but does not provide any access control.'),
];
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | allow-configurable-filesystems_3200974-6.patch | 8.07 KB | kalpaitch |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
kalpaitch commentedComment #3
kalpaitch commentedComment #4
kalpaitch commentedRelated to https://www.drupal.org/project/vbo_export/issues/2988257
Comment #5
kalpaitch commentedLet me know your thoughts. The default option is 'private', if other options are available they can be selected, with a fall back to 'public'.
If you're happy with this approach I'll put some thought into how best to make this setting backwards compatible.
Comment #6
kalpaitch commentedComment #8
eli-tPatch in #6 unfortunately does not apply against 8.x-1.5.
Comment #10
eli-tMR 95 applies against 8.x-1.8 and manual testing is successful, but there are automated test failures.
Comment #11
eli-tAutomated tests all now pass.
This would require further work if it were to be merged - there would need to be a config update to change existing config to use the new schema. But putting it to needs review to get the opinion of a maintainer if this is a viable addition before doing that additional work.
Comment #12
eli-tPutting back to Needs work after discussing and agreeing with Steven Jones in Drupal Slack this would be a good change.
Comment #13
eli-tThis now has a post update function to cover the schema update with full kernel tests. Setting to needs review.
Comment #14
steven jones commentedThis looks super nice @eli-t thanks, I'll review this properly later this week, but at first glance I can't really see anything amiss.
Comment #15
steven jones commentedOkay, so actually, 'later this week' turned out to be a few minutes ago. Hah!
I just want to discuss being a bit more strict about missing filesystems etc. and see if you think that would be a problem for your use-case for example?
Comment #16
eli-tComment #17
steven jones commentedReckon this is RTBC now..
Comment #19
eli-tWoo! Thanks @steven-jones and thanks more to @kalpaitch for writing the initial patch in the first place.
Comment #20
steven jones commentedThanks for the work here, it'll go out in the next release.