Moving Versioned extension File application to Assets Admin

[alessandromarotta]

@see https://github.com/silverstripe/silverstripe-asset-admin/pull/1340

[GuySartorelli]

See https://github.com/silverstripe/silverstripe-asset-admin/pull/1340#issuecomment-1483764423

[alessandromarotta]

@GuySartorelli I see this PR has been closed, but my proposal was to remove the automatic versioning of the File class. If Assets can work without versioning, why should I be forced to have versioning for the File class if I'm interested in installing the Versioned module for other DataObjects? It doesn't make sense to me. Instead, the Assets Admin module requires - de facto - the Versioned module, practically nothing works without the Versioned module, neither exploring the gallery. Therefore I believe that the Asset Admin module is the best candidate to host the application of the Versioned extension to the File class and not the Versioned module.

[GuySartorelli]

The implementation you proposed will still result in versioning assets by default - you just changed which module adds the extension to one which currently theoretically has no dependency for versioned, adding additional coupling and therefore complexity. If there is currently some assumption in the assets admin module that assets will be versioned, we should work to reduce that coupling not re-enforce it.

If you want to be able to override this default application of the extension, I recommend creating a PR that adds a key for the extension which you could then set to null in your own project.

I.e.

 ---
 Name: versionedfiles
 ---
 SilverStripe\Assets\File:
   extensions:
-    - SilverStripe\Versioned\Versioned
+   Versioned: SilverStripe\Versioned\Versioned

You could then use this yaml in your own project:

---
After: versionedfiles
---
SilverStripe\Assets\File:
  extensions:
    Versioned: null

[alessandromarotta]

Thanks for you reply but I disagree: the Asset Admin module is not mandatory if I only use the Admin module (which has the Versioned module dependency). If you are interested in using only Admin without Assets Admin, you can do it and my proposal would be a solution.

If one day Assets Admin works without the Versioned module, then it will be different. But today this is not the case and there is no alternative solution other than writing your own module to manage Assets without the Versioned module.

[GuySartorelli]

there is no alternative solution other than writing your own module to manage Assets without the Versioned module.

I provided an alternative solution in my previous comment.

The expected status quo currently is that assets will be versioned by default if the versioned module is installed. I don't think that there is sufficient reason here to change that status quo.

Given that you feel strongly about this I'll tag @silverstripe/core-team to see if there is anyone in that group who agrees with your approach - but my opinion is that this setting should remain where it is and could be made easier to disable with the alternative approach I recommended in my previous comment.

[alessandromarotta]

@GuySartorelli with your solution, I get this when I try to see an image through the Assets Admin

Currently Assets Admin requires the Versioned module applied to File class (but there's not dependency in its composer.json)

[GuySartorelli]

This all came about because you want to avoid having the versioned extension applied to your files when you don't have the assets admin module, didn't it?

[emteknetnz]

I think the way the config now is setup is good and I agree with Guy here to not change the status quo

It seems like a very narrow use case to version all the DataObjects, but then exclude Files specifically. I don't see much likelihood of any near term work being done to decouple asset-admin so that it works with unversioned Files

Since you're not going to get a product level solution here, you'll need project level solutions if you wish to continue with unversioned Files. You've indicated that Guy's solution doesn't work out of the box with asset-admin. You might be able to find some other workaround, such as creating an onAfterWrite extension hook implementation for File that publishes the file.

[alessandromarotta]

@GuySartorelli Exactly, the point is that I may want to install only the Admin module, but without the AssetsAdmin module. And I could need the Versioned module only for some DataObject (some and not all as @emteknetnz said). I know that I can deactivate the Versioned module for Files with removeExtension(Versioned::class) but for me it doesn't make sense that in the Versioned module the versioning is automatically activated for the Files.

What does the Versioned module have to do with the Assets module? The Assets module can work just fine without the Versioned module. Applying versiong for file in the Versioned module just doesn't make any logical sense.

Instead it makes sense, and indeed it is required, that the versioning for Files is active and present when I need the Assets-Admin module (because the Assets-Admin module - currently - is deeply coupled with the Versioned module code). @emteknetnz I'm not asking to have Assets-Admin decoupled. I'm asking to have no versioning for Files by default, because there's not reason to do it.

Assets doesn't require versioning but the Versioned module (installed by the Admin module) force you to have it.

For this reason I suggested to move the Versioned extension on File from the Versioned module to Assets-Admin module, which requires it. You can do it, no risk to breaking code with this proposed change.

[GuySartorelli]

You're right that your proposed change doesn't break any code - but it does break expectations. Given that the beta for CMS 5 was released ages ago and we're planning to release the release candidate soon, it's a hard sell to have such a large change of expectations this late - especially given we have already told you how to turn off versioning for your use case.

I'm going to stop responding here now as we're just going around in circles. The core team has been tagged so if any of them agree with your approach we can revisit it then.

[michalkleiner]

The issue at hand presents several separate things in my view:

• what module applies the Versioned extension to File?
• is the versioned module a soft or hard dependency of the assets module?
• is the versioned module a soft or hard dependency of the assets-admin module?
• current status quo in terms of expectations which module provides what and any hidden interdependencies

We can say that adding the Versioned extension to the File in the Versioned module without anything else is a bit strange, but it's what people are used to. Also, it's not ideal that assets-admin doesn't function without Versioned and doesn't specify it as its dependency in composer.json OR doesn't correctly work without it.

From my perspective, the end goal would be to have everything working without versioning, as it was probably intended at the very beginning. Then asset versioning could be applied through a separate module since we don't have an ideal place for the config itself, or have it in a recipe etc.

Since there is a workaround available (PR https://github.com/silverstripe/silverstripe-versioned/pull/400 merged) for people wanting to remove versioning from assets I'd say we can either revisit this later on (we can create an RFC issue on the meta repo after CMS5 is stable) or leave as is as I don't think this is a huge pain point/a priority for anyone.