GridFieldBulkEditingTools icon indicating copy to clipboard operation
GridFieldBulkEditingTools copied to clipboard

FIX: asset-admin is a suggested, not required module

Open sminnee opened this issue 3 years ago • 5 comments

This lets you install the module in apps that don’t have asset-admin (such as ModelAdmin-focused apps, where batch editing would be handy)

Fixes #202.

sminnee avatar Aug 05 '20 01:08 sminnee

Any thoughts on this?

sminnee avatar Aug 15 '20 22:08 sminnee

@robbieaverill @colymba do you think that this will cause issues for people who expect to have asset-admin pulled in for them? I'd note that I haven't tested this module on an installation lacking asset-admin, and while I'd like to, it's difficult to pull it into a project without this PR.

sminnee avatar Sep 10 '20 20:09 sminnee

It's a tricky thing to answer really, because this module provides two equally valuable features: bulk uploading, and bulk editing. If you use this for the bulk uploading then maybe you would expect asset-admin to be installed. I have a feeling that people are more likely to install this on a full Silverstripe installation though, so this PR seems sensible to me.

robbieaverill avatar Sep 10 '20 21:09 robbieaverill

Just had a look through the codebase. There's one concrete reference to AssetAdmin and a few references to File and Image. I think there should probably be some logic to not load the classes that depend on silverstripe/assets and silverstripe/asset-admin if we're going to remove those as a dependency.

GuySartorelli avatar Jun 28 '22 02:06 GuySartorelli

Fair enough, the assumption was that BulkUploadHandler wouldn't be used on a project without AssetAdmin, but it can probably be patched to provide an error message to that effect if you try and use it.

sminnee avatar Jul 02 '22 07:07 sminnee