Disabling a scheduler field gives undefined index: unpublish_on in scheduler_form_node_form_alter()

The solution in #4 gets rid of the error, but the form elements are still being populated. It seems like it'd be better to just skip the entire form_alter altogether if either of the elements are hidden.

I'm attaching a patch that will just simply bail if the components aren't displayed. At the very least an easier way to get the display info is:

$display = $form_state->getFormObject()->getFormDisplay($form_state);

And then you can check each component with:

$display->getComponent('unpublish_on');

or...

$display->getComponent('publish_on');

Sorry I had a typo, here is the right patch.

Thanks!

Thanks for the work and the patch. However, are you sure that it is correct to bail out with an OR test? That means if either of the components are not displayed we process no further.

We also need test coverage for this.

Here is patch which adds a test to make sure that a disabled scheduler field is not displayed, that the non-disabled field is still displayed, and that the undefined index error does not occur. This will fail because this patch only includes the new test, and not any code fix from scheduler_form_node_form_alter().

I am also attempting, via a change to the newly added drupalci.yml, to only run this one test class. This might need a re-run if I have not got the syntax right.

No tests were run because the class name is case-sensitive:
Drupal\Tests\scheduler\functional\SchedulerFieldsDisplayTest
should have been
Drupal\Tests\scheduler\Functional\SchedulerFieldsDisplayTest

Good. Just the single test class ran, and we got the required error as expected:

There was 1 error:
1) Drupal\Tests\scheduler\Functional\SchedulerFieldsDisplayTest::testDisabledFields
Exception: Notice: Undefined index: publish_on
scheduler_form_node_form_alter()() (Line: 181)

Here is the new test, with the added change from #7. As I mentioned in #10 I don't think this will solve the problem properly.

We can't just bail out at the top if either of the fields are disabled. We can bail out if both are disabled, but need to cater for the case when one is and one is not.

That's good. Now, just to check the complete test run.

If anyone would like to test this patch, you will need to download the latest dev, as there have been other recent changes. When done please mark the issue RTBC then I will commit it.

Jonathan

... and without the coding standards warnings

Works for me!

Fixed and committed. Thanks everyone.

I had a request from a user who is running Scheduler 1.0 (not the dev version) and would like to apply this fix. So here is a modified patch which applies to 8.x-1.0. It just fixes scheduler.module, there was no need to add the new test to this patch.