Display webform tab only on webform enabled content type nodes

Nice work!

Just a quick code review at the moment, I'll try testing the actual patch later:

1. +++ b/src/Form/WebformComponentsForm.php
   @@ -31,10 +31,10 @@ class WebformComponentsForm extends FormBase {
   -  public function buildForm(array $form, FormStateInterface $form_state, NodeInterface $node = NULL) {
   +  public function buildForm(array $form, FormStateInterface $form_state, NodeInterface $webform_node = NULL) {
        $form = [
          '#tree' => TRUE,
   -      '#node' => $node,
   +      '#node' => $webform_node,
   

   Somewhat tempting to create a local variable $node (i.e. $node = $webform_node) rather than using $webform_node. However given $webform_node is what the ParamConvert expects/returns then I guess leaving it as that is ok.

2. +++ b/src/ParamConverter/WebformParamConverter.php
   @@ -0,0 +1,40 @@
   + * Parameter converter for upcasting entity ids to full objects.
   

   Comment needs to be updated.

3. +++ b/webform.module
   @@ -1732,7 +1732,7 @@ function webform_user_role_delete($role) {
   -  if (isset($form['#node']->type) && $form_id == $form['#node']->type . '_node_form' && \Drupal::config('webform.settings')->get('node_' . $form['#node']->type)) {
   +  if (isset($form['#node']) && $form['#node']->bundle() && $form_id == $form['#node']->bundle() . '_node_form' && \Drupal::config('webform.settings')->get('node_' . $form['#node']->bundle())) {
   

   Good catch with $node->type => $node->bundle(). There are a few other instances throughout the module, can you split this change off into another issue and we can get it fixed quickly.

4. +++ b/webform.routing.yml
   @@ -13,14 +13,16 @@ webform.settings:
   -    # @todo This needs to call/replicate webform_node_update_access().
        _permission: 'edit webform components'
   +    _entity_access: 'webform_node.update'
   

   We're not creating our own custom entity type here so unless I'm mistaken this is not correct. Also webform_node_update_access() does quite a few additional checks. I think for simplicity lets remov this and keep this patch focused on showing the webform tab on webform enabled content types.

As for not seeing the tab on webform nodes, again I haven't tested the path, but I'd look to something being amiss in webform.links.task.yml

Thanks.

1. I've gone ahead and fixed ->bundle() in #2504477: Use $node->bundle() instead of $node->type so it'll need to be removed here.

2. +++ b/src/Form/WebformComponentsForm.php
   @@ -218,7 +218,7 @@ class WebformComponentsForm extends FormBase {
   @@ -226,7 +226,6 @@ class WebformComponentsForm extends FormBase {
   
   @@ -226,7 +226,6 @@ class WebformComponentsForm extends FormBase {
          '#weight' => -1,
        ];
        webform_input_vars_check($form, $form_state, 'components', 'warning');
   -
        return $form;
      }
    
   

   Unrelated change, please remove.

3. +++ b/src/ParamConverter/WebformParamConverter.php
   @@ -0,0 +1,40 @@
   + * Parameter converter for to access the webform enabled node only.
   

   Grammar is off. How about: "Parameter converter for upcasting webform node ids to full objects."

4. +++ b/webform.routing.yml
   @@ -13,14 +13,15 @@ webform.settings:
   -    # @todo This needs to call/replicate webform_node_update_access().
   

   This comment needs to stay.

5. I've still not tested the patch, how'd you go getting the tab to display?

Needs work for showing the tab.

Also #7 4. still needs to be addressed.

Thanks for your work here @sumitmadan

I'm still seeing the webform tab on node types that aren't webform-enabled. I'm not 100% certain I'm testing correctly:

1) Downloaded Drupal-beta14 (drush)
2) Performed fresh install, standard profile
3) Cloned the webform repository, and checked out the 8.x-4.x branch
4) Downloaded the patch from #12
5) Applied it using git
6) Ran drush cache-rebuild

It looks to me like there was a misplaced ! on the check for the node setting: WebformNodeAccessCheck.php:42. Removing the ! from the second condition of that if statement seems to have the desired result for me.

Committed and pushed to 8.x-4.x. Thanks!

In the interest of forward progress I've gone ahead and committed #15 with minor changes. Thanks for your work here @sumitmadan.

1. I restored the @todo from #7 4.
2. Can already see that this will need refactoring to allow other paths to use this (like node/{node}/webform/conditionals). That will also then need to access checking in webform_node_update_access().