localgovdrupal / localgov_workflows

Default editorial workflow for LocalGov Drupal content.
GNU General Public License v2.0
0 stars 1 forks source link

Should this module or other modules take responsibility for adding scheduled transitions #111

Open andybroomfield opened 1 month ago

andybroomfield commented 1 month ago

Noting an issue we discovered with https://github.com/localgovdrupal/localgov_job_vacancies/issues/1 in that we can't assign permissions to scheduled transitions, becuase the job_vacancies arn't added as an enabled type of scheduled transitions.

If the modules are installed in the order localgov_job_vacancies and then localgov_workflows, job_vacancies are added as an enabled type and the permissions are correctly assigned. It does not work the other way round.

This is because localgov_workflows enables scheduled transitions for all content types, but only during hook_install.

If a module currently wants to opt in to scheduled transitions after localgov_workflows is installed, then it needs to opt in seperatly, like localgov_blogs does. Note, this is the only version I can find, other modules do not have an implementation though localgov_job_vacancies will be adding support.

This leads to the question of what is the correct approach here, as either localgov_workflows or all other modules providing content types are going to need to provide an implementation.

markconroy commented 1 month ago

How about we add a submodule for job_vacancies called "Job Vacancies Scheduled Transitions" and if you enable that it adds job vacancies to the scheduled transitions set up?

Though that might have the issue that if scheduled transitions isn't already enabled, this will enable it and set it for all content types.

ekes commented 1 month ago

Surprised really we usually do changes in hook_install and hook_modules_installed so that it doesn't matter the order no?

Ah because it doesn't know if the module added content types. It could just look I guess.

I have to say we've had to remove workflows from content types as it grabs everything with a localgov_ prefix and sometimes you don't actually want it to!

finnlewis commented 1 month ago

Discussing in Tech Drop-in.

The fact that localgov_blogs takes care of it's addition of scheduled transitions in hook_install and hook_modules_installed makes that a very attractive approach.

However, that approach also means that if someone removed the scheduled transition for blogs, when another module gets installed, blogs will get it's scheduled transition back!

Hmmm... tricky!

andybroomfield commented 1 month ago

Localgov_blogs checks to see if it is scheduled_transitions that is being installed before proceding. https://github.com/localgovdrupal/localgov_blogs/blob/87288fe9c062860956aa826c868faaf7ede883c9/localgov_blogs.module#L22-L24

stephen-cox commented 1 month ago

I did have the thought of creating an admin page for workflows that automatically does the scheduled transition and Drupal workflow configuration. This could expose an API to do the config automatically, making it cleaner for a module to opt in during an install.