Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility with advancedworkflow #15

Closed
chillu opened this issue Nov 28, 2017 · 2 comments
Closed

Compatibility with advancedworkflow #15

chillu opened this issue Nov 28, 2017 · 2 comments

Comments

@chillu
Copy link
Contributor

chillu commented Nov 28, 2017

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):

  • Model should store separate "desired" dates (yet to be approved) from actual states (see WorkflowEmbargoExpiryExtension.php)
  • Only the approval should trigger a queued job (and the default embargo-expiry module behaviour). See PublishItemWorkflowAction
  • Hide schedule date fields if "desired" dates are set (should be handled via standard "cancel workflow" request)
  • Define if record can be edited during embargo in workflow action (see PublishItemWorkflowAction)
  • Migration path shouldn't be required, since the new module keeps the same data structures

Existing discussions:
symbiote/silverstripe-advancedworkflow#47
symbiote/silverstripe-advancedworkflow#187

@chillu
Copy link
Contributor Author

chillu commented Nov 28, 2017

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
Copy link
Member

@chillu I believe support for Workflow should be possible once I have merged:
#28

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

I've added matching fields:

  • DesiredPublishDate
  • DesiredUnPublishDate
  • PublishOnDate
  • UnPublishOnDate

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants