silverstripe-terraformers / silverstripe-embargo-expiry

BSD 3-Clause "New" or "Revised" License
7 stars 7 forks source link

Compatibility with advancedworkflow #15

Closed chillu closed 6 years ago

chillu commented 6 years ago

This module essentially duplicates a subset of the features in https://github.com/symbiote/silverstripe-advancedworkflow/, without coupling them to workflow concerns. Ideally we wouldn't need this duplication - the module maintainer Marcus agrees that we should split out the feature as well.

Missing features in new module (or bridge to advancedworkflow):

Existing discussions: https://github.com/symbiote/silverstripe-advancedworkflow/issues/47 https://github.com/symbiote/silverstripe-advancedworkflow/issues/187

chillu commented 6 years ago

I hope we can do this in a minor release of the advancedworkflow module, which means we could introduce this feature in the CWP 2.x release line at some point next year. The data model stays the same, it's just the extension names which change (and we can fix that through subclasses). YAML config keys would need to be adjusted. So we'd add this module to our supported modules under CWP, instead of maintaining the same code twice.

chrispenny commented 6 years ago

@chillu I believe support for Workflow should be possible once I have merged: https://github.com/silverstripe-terraformers/silverstripe-embargo-expiry/pull/28

Model should store separate "desired" dates (yet to be approved) from actual states

I've added matching fields:

Only the approval should trigger a queued job

As part of ensurePublishJob() and ensureUnPublishJob() there are now extension points (publishJobCanBeQueued and unPublishJobCanBeQueued) which the Workflow module could use to prevent the creation of QueuedJobs outside of any context that they desire. The Desired..Date fields will still hold their respective values, and the Workflow module can then "take over" and do their own thing, or simply call ensurePublishJob() and ensureUnPublishJob() again with publishJobCanBeQueued and unPublishJobCanBeQueued now allowing Job creation.

Hide schedule date fields if "desired" dates are set

I don't think there is anything for me to change here. I'm exposing fields that are relevant given the current Object state and whether or not editing during embargo is allowed.

Define if record can be edited during embargo in workflow action

As part of this module's isEditable() method, there is now an extension point available (embargoRecordIsEditable) which Workflow could use to add their own constraints for canEdit()/canPublish().

Migration path shouldn't be required

I believe this is true. As you say, they use the same fields.