When saving a node that has a scheduled publish time but not unpublish time (because the node type is not configured to have one), I get this error:

notice: Undefined property: stdClass::$unpublish_on in /shared/vash/sandboxes/garfield/drupal6/www/sites/all/modules/scheduler/scheduler.module on line 583.

Looks like a case of not checking to see if such a variable would even be set. Should be a simple fix. I have not checked to see if the issue still exists in Drupal 7.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jonathan1055’s picture

Title: NOTICE error » Undefined property $unpublish_on during update

Hi Crell,
Yes at first glance is does appear to be a bug and simple to fix. But what I can't understand is how it has not been found and reported before. I took a break from supporting scheduler when the options were added to enable publishing and unpublishing independently, so I will have to check back with the code, but I still can't work out how no-one (including me) has not spotted this since then.

Patch coming up soon,

Jonathan

jonathan1055’s picture

Version: 6.x-1.x-dev » 6.x-1.8

Just checked the actual code line number, and I think you must be using 6.x-1.8 real (as of 27th Dec 2010), not the -dev version. Please can you confirm this?

Crell’s picture

Yes I was using the stable. Just what Drush downloaded when I asked for it.

Many D6 servers are not configured for E_ALL by default, but my dev box is. It actually takes work to suppress notices for me. :-) In D7, Drupal itself forces E_ALL anyway.

jonathan1055’s picture

Status: Active » Needs review
FileSize
2.08 KB

Hi,
Yes, of course most time E_ALL is not enabled. I use the devel module, and yes there are many other notices which are shown for all kinds of other modules - including some big and important modules!

Here is a patch, against the dev release of 15th May. If this sorts it, then I know we will have to fix the D7 version first, but I thought given as you are using D6 you should get the patch first.

Jonathan

Eric-Alexander Schaefer’s picture

So this problem has always been there we just did not see the warning?

Crell’s picture

Eric: That's common with E_NOTICE errors. That's why you should always develop under E_ALL. :-)

jonathan1055’s picture

I've checked the D7 code and this is not a problem, because it uses the EMPTY function, which does
not cause an error if the property is undefined.

Here is a slightly tidier patch for D6. Tested when updating a node type which has just publishing enabled, and
on another which has just unpublishing enabled, both already having a date set. Works ok for me, no errors,
but would be good for someone else to confirm.

jonathan1055’s picture

Version: 6.x-1.8 » 6.x-1.9

Retested my patch against 6.x-1.9+4 (dev 28th July 2013). Patch applies ok (with an offset due to other changes).

notice: Undefined property: stdClass::$unpublish_on in scheduler_nodeapi()
(line 648 of /Library/WebServer/Documents/drupal6/sites/all/modules/scheduler/scheduler.module)

and

notice: Undefined property: stdClass::$publish_on in scheduler_nodeapi()
(line 656 of /Library/WebServer/Documents/drupal6/sites/all/modules/scheduler/scheduler.module)

Both of these are fixed by the patch. Can someone else verify, then mark it RTBC.

Jonathan

Crell’s picture

Code looks decent, but I have no D6 site around to test it with to verify.

pfrenssen’s picture

I just checked if this bug also occurs on 7.x and higher but it does not. hook_nodeapi() has been replaced with hook_node_update() and the rewritten code uses !empty() which does not throw this notice.

Status: Needs review » Needs work

The last submitted patch, _scheduler.1235920_7.undefined_property_during_update.patch, failed testing.

jonathan1055’s picture

Yes, see my comment in #7 Drupal7 is fine, this fix is only required for D6.
I will re-roll the patch in a/ b/ format then re-submit for testing.

jonathan1055’s picture

Version: 6.x-1.9 » 6.x-1.x-dev
Status: Needs work » Needs review
FileSize
2.17 KB

Here is the same code changes as in the patch in #7 but in a/ b/ format against the latest D6 dev

pfrenssen’s picture

Status: Needs review » Fixed

Makes sense, committed: ad6bfba.

Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.