Workflow state and transition add/edit form IDs have a '-' in them

I wonder if the FormHandler should just replace hyphens with underscores when firing the hooks. Not sure if that would be an API change given technically new hooks would be firing.

Another option would be just changing the hyphens to underscores.

The convention elsewhere seems to be hyphens. Re suggestion in #3, making this part of EntityForm makes sense to me.

So it looks like we're moving this out to be a standard entity system issue.

#10 seems fine to me, as long as test bot agrees.

Back to RTBC, only test-only patch failing.

RTBC+1 @sam152 You are right. Just a piece of advise, try to upload test-only patch before the full patch, this way Status won't be set to Needs Work , I suppose.

Committed and pushed 973e6b4 to 8.4.x and 3a18f95 to 8.3.x. Thanks!

Probably my mistake.

Committed to 8.3.x since workflow is experimental.

I got this wrong - we shouldn't be changing the EntityForm implementation. That's unnecessarily risky - we should be doing the same as \Drupal\workflows\Form\WorkflowTransitionDeleteForm::getFormId(). Sorry for the false commit.

  /**
   * {@inheritdoc}
   */
  public function getFormId() {
    return 'workflow_transition_delete_form';
  }

Is what \Drupal\workflows\Form\WorkflowTransitionDeleteForm::getFormId() does

@alexpott - You think we should just hardcode the form IDs?

I quite liked the idea in #10 as this solves the issue across the whole of core and adds a test for it too.

@timmillwood I think it is an okay idea but I think that the fix is not 8.3.x because it is a behaviour change. So let's do the quick fix here and open a followup to discuss the entity form fix with the entity maintainers.

Opened #2856645: Operations with hyphens should create entity form IDs with underscores.

This patch has form IDs consistent with the current defined ones and the form class names, however it's contrary to how the entity form would have generated the UI using the opperation. workflow_state_edit_form vs workflow_edit_state_form.

The patch looks good from the Workflow point of view.

Don't think we need a test for this.

Committed and pushed eb83216 to 8.4.x and 82f2015 to 8.3.x. Thanks!

As this is an experimental module backported to 8.3.x