symbiote / silverstripe-advancedworkflow

A highly configurable step-based workflow module.
BSD 3-Clause "New" or "Revised" License
47 stars 71 forks source link

Published content via WorkflowEmbargoExpiryExtension without QueuedJobs Module results in error. #167

Open phptek opened 10 years ago

phptek commented 10 years ago

Witthout installing the QueuedJobs module, logic in WorkflowEmbargoExpiryExtension will throw errors when content is attempted to be passed to a "PublishItemWorkflowAction".

Class 'AbstractQueuedJob' not found in htdocs/osdev/advancedworkflow/code/jobs/WorkflowPublishTargetJob.php on line 7
phptek commented 10 years ago

There are a couple of solutions here.

1). Use class_exists (Fugly)

if(class_exists('AbstractQueuedJob')) {
    class WorkflowEmbargoExpiryExtension extends DataExtension {
    ...
    }
}

2). Define silverstripe-queuedjobs as a composer dependency

What were/are the reasons for embargo/expiry being defined as an extension class instead of default functionality? If it were default, queuedjobs could be included as a composer dependency and the problem would be resolved. Of the 2 or 3 decent-sized clients I've spoken with, they could never imagine a workflow feature without embargo and expiry.

3). Something else

I had thought of an SS config solution, but unless this were defined in the module's own config, not just in mysite, it's almost the same as just composerifying with queuedjobs as in "..you need queuedjobs for this to work.."

::yml
---
Only:
    moduleexists 'AbstractQueuedJob'
---
SiteTree:
    extensions:
        - WorkflowEmbargoExpiryExtension
tractorcow commented 10 years ago

I prefer using the below to get around classes that should only be used in some situations:

if(!class_exists('AbstractQueuedJob')) return;

class WorkflowEmbargoExpiryExtension extends DataExtension {
    ...
}

It means less indentation dramas and :hurtrealbad:

phptek commented 10 years ago

Yeah for sure, early returns win every time. It was a hastily typed solution (but it's still ugly).

robbieaverill commented 7 years ago

Has this been done? We could also apply the extension in a moduleexists YAML conditional