The FALSE value for #group FAPI property does not have expected behaviour

Sending to testbot

Thanks for the explanation and the patch, which looks perfectly reasonable. I'd like my co-maintainer to check this with 8.x first, before any changes are committed for 7.x.
Jonathan

@hugronaphor I can confirm that we allow failing patches to be merged after review and manual testing until our test suite is ported to D8 in #2594615: Automated testing in 8.x [meta].

The use cases explained in #6 are very interesting. There are some serious usability bugs there. It would maybe still be a good idea to add some test coverage for these cases.

1. +++ b/scheduler.edit.inc
   @@ -39,26 +39,31 @@ function _scheduler_form_alter(array &$form, FormStateInterface $form_state) {
   +
   +  if ($use_vertical_tabs && ($publishing_enabled || $unpublishing_enabled)) {
   

   Can you add the reasoning for this if in a line of documentation? We have the habit of documenting every switch.

2. +++ b/scheduler.module
   @@ -110,19 +110,10 @@ function scheduler_form_node_type_form_alter(array &$form, FormStateInterface $f
   -  // Load the real code only when needed. First check that the user has
   -  // permission to use Scheduler.
   +  // Load the real code only when the user has permission to use Scheduler.
      if (\Drupal::currentUser()->hasPermission('schedule (un)publishing of nodes')) {
   -    // Check if scheduling has been enabled for this node type.
   -    /** @var \Drupal\node\NodeTypeInterface $type */
   -    $type = $form_state->getFormObject()->getEntity()->type->entity;
   -    $publishing_enabled = $type->getThirdPartySetting('scheduler', 'publish_enable', FALSE);
   -    $unpublishing_enabled = $type->getThirdPartySetting('scheduler', 'unpublish_enable', FALSE);
   -
   -    if ($publishing_enabled || $unpublishing_enabled) {
          module_load_include('inc', 'scheduler', 'scheduler.edit');
          _scheduler_form_alter($form, $form_state);
   -    }
      }
   

   Why has this been removed? Is this related to this issue?

   Not that I necessarily disagree with this simplification, but it might be better handled in a different issue. We might even get rid of the scheduler.edit.inc file completely. It now only contains this particular form alter, doesn't seem very useful any more to split this off in a separate file.

   edit: Oh I see, you have explained the reasoning in comment #6.

Assigning to me to address the remarks from #15 and commit.

The problem for the non-scheduled content types was reported in #2625572: Admin setting 'Enable content type for scheduler' is ignored, but since this is fixed here as a by-product of fixing the #group property I have marked that issue as a duplicate of this issue.

Added the documentation to the switch and committed. Thanks a lot for fixing this!

Yes, I will do the 7.x fix. I think the patch in #2 looks OK?

Tested manually and via simpltest. All fine.
Now committed to 7.x. Thanks hugronaphor for the original patch.