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.

Files: 
CommentFileSizeAuthor
#13 _1235920_13.undefined_property_during_update.patch2.17 KBjonathan1055
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#7 _scheduler.1235920_7.undefined_property_during_update.patch2.14 KBjonathan1055
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch _scheduler.1235920_7.undefined_property_during_update.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#4 _1235920_4.scheduler.undefined_property_during_update.patch2.08 KBjonathan1055
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch _1235920_4.scheduler.undefined_property_during_update.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

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
StatusFileSize
new2.08 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch _1235920_4.scheduler.undefined_property_during_update.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

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 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

StatusFileSize
new2.14 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch _scheduler.1235920_7.undefined_property_during_update.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

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
StatusFileSize
new2.17 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

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.