Move buildStateConfigurationForm/buildTransitionConfigurationForm to WorkflowTypeFormInterface.

Lets see what tests break.

+++ b/core/modules/workflows/src/Form/WorkflowStateAddForm.php
@@ -43,10 +44,13 @@ public function form(array $form, FormStateInterface $form_state) {
+    if ($workflow->getTypePlugin() instanceof WorkflowTypeFormInterface) {
+      $form['type_settings'] = [
+        $workflow->get('type') => $workflow->getTypePlugin()

+++ b/core/modules/workflows/src/Form/WorkflowStateEditForm.php
@@ -62,10 +63,13 @@ public function form(array $form, FormStateInterface $form_state) {
+    if ($workflow->getTypePlugin() instanceof WorkflowTypeFormInterface) {
+      $form['type_settings'] = [

+++ b/core/modules/workflows/src/Form/WorkflowTransitionAddForm.php
@@ -62,10 +63,13 @@ public function form(array $form, FormStateInterface $form_state) {
+    if ($workflow->getTypePlugin() instanceof WorkflowTypeFormInterface) {
+      $form['type_settings'] = [
+        $workflow->get('type') => $workflow->getTypePlugin()

+++ b/core/modules/workflows/src/Form/WorkflowTransitionEditForm.php
@@ -78,10 +79,13 @@ public function form(array $form, FormStateInterface $form_state) {
+    if ($workflow->getTypePlugin() instanceof WorkflowTypeFormInterface) {
+      $form['type_settings'] = [
+        $workflow->get('type') => $workflow->getTypePlugin()

Maybe we should extract getTypePlugin into a variable in all the cases here.

Otherwise, I like it!

+1 RTBC, but probably should have someone else review this based on #2.

Minor OCD, we already have 7 instances of $workflow_type, none of $workflow_type_plugin + updating the formatting to be on one line.

Re: #11.2, the idea here is not forcing plugins to implement WorkflowTypeFormInterface, with empty implementations where forms aren't required. We're already checking the type of plugin the workflow type is to ensure \Drupal\Core\Plugin\PluginFormInterface::buildConfigurationForm is optional, this issue is just giving state and transition forms the same treatment.

A similar concept exists with ImageEffectBase and ConfigurableImageEffectBase. I guess the trade off here is, do we want plugins implementing methods that are potentially irrelevant or segregate the interfaces accordingly and check those types where relevant.

Discussed this with @larowlan. It was suggested to add a method on the base called hasForms or something like that. Having thought about this, I'm in two minds about this.

One one hand it makes the code a bit cleaner, instanceof checks are generally associated with anti-patterns so it's nice to move that decision somewhere else. On the other hand, it doesn't actually enforce at a language level that the set of methods we need from the interface are actually going to be implemented. A plugin implementation could technically return TRUE from the method, while not implementing the interface. Maybe a sign that a convention over a language feature is a smell. It also adds a method to the "simple" workflow implementation that doesn't really make sense in isolation. Why does a "hasForms" method exist in a context where I never expected a form in the first place?

Maybe a final method on the base which checks static::class implements the right interface would alleviate the first issue for me, but it's a coin toss for me. I probably lean towards simply using instanceof checks where appropriate and deferring to the pattern we already have.

NW for consensus.

I think we're going to need this even more once #2896722: Leverage PluginWithFormsInterface to encapsulate @WorkflowType schema and clean up state/transition form methods. is done.

Closing in favour of the new approach outlined in #2896722: Leverage PluginWithFormsInterface to encapsulate @WorkflowType schema and clean up state/transition form methods.