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

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

Open
andybroomfield opened this issue Aug 29, 2024 · 5 comments
Labels
question Further information is requested

Comments

@andybroomfield
Copy link
Contributor

Noting an issue we discovered with localgovdrupal/localgov_job_vacancies#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.

@andybroomfield andybroomfield added the question Further information is requested label Aug 29, 2024
@markconroy
Copy link
Member

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

ekes commented Aug 29, 2024

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

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
Copy link
Contributor Author

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants