Add canPublish check for ChangeSet

[mark-a-j-adriano]

Maybe the solution for #383

[kinglozzer]

We shouldn’t add this kind of logic here, as it’ll potentially block programatic publishing (e.g. a CLI task where no user is logged in)

[mark-a-j-adriano]

ChangeSetItem already has its own canPublish method that checks the record object's canPublish, so it must be something else going on (if something is actually going on).

Nope, ChangeSetItem canPublish method is not called.

silverstripe-versioned/src/ChangeSetItem.php

Line 307 in 4baa7b4

$object->publishSingle();

Already acts on the asset

[mark-a-j-adriano]

@kinglozzer -> not sure where to put the logic if it is not the correct place. (In our use case this might suffice)

[kinglozzer]

I think for your use case I’d probably remove the image from $owns and add some logic in onAfterPublish() to check the date and publish if applicable

[michalkleiner]

That would be a workaround for Mark, but I think this is a legit bug where it is reasonable to anticipate the permissions are respected, or we openly document that ownership cascade publishing trumps individual object permissions.

[GuySartorelli]

I think this is a legit bug where it is reasonable to anticipate the permissions are respected

I agree

[kinglozzer]

Yep sorry I meant as a workaround! Will post my thoughts in the issue #383

[michalkleiner]

ChangeSetItem already has its own canPublish method that checks the record object's canPublish, so it must be something else going on (if something is actually going on).

[GuySartorelli]

I'm going to close this PR for now. As mentioned in the issue, this can't be resolved until we're ready to start working on a new major release.
The closed PR will still be associated with and linked from the issue, which is already in the CMS 6 milestone, so when we start working on that we'll have access to this PR to reference as a potential solution.