Empty Expiration Date Field is filled with bad information!
wilbersmith - March 18, 2009 - 03:01
| Project: | Node Expire |
| Version: | 6.x-2.03 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Description
Problem is like that addressed in:
http://drupal.org/node/366499
Where upon creation of a node where expiration date field has no default the date is it filled with "1970-01-01".
The solution in the forum above fixed that issue but now upon editing the node that date repopulates the field automatically every time.
I've tried to look through to see how I can change it as it seems quite simple but I am a hobby programmer and need to orientate myself to drupals way of doing things. Following is my solution, please correct this if it is too much of a hack:
from line 12 of node_expire.nodeapi.inc:
function _node_expire_nodeapi(&$ntypes, &$node, $op, $a3 = NULL, $a4 = NULL) {
switch ($op) {
case 'load':
$query = db_query('SELECT expire, expired FROM {node_expire} WHERE nid = %d', $node->nid);
// Use the existing expiration data if present.
if ($row = db_fetch_object($query)) {
$node->expire = $row->expire;
$node->expired = $row->expired;
}
// My solution is to make it behave like it does on node creation...
if ($node->expire == '0'){
unset($node->expire);
}
break;
case 'prepare':
//patch code begins, make sure we don't pass an empty variable to the format date function.
if(!empty($ntypes['default'])){
$node->expire = format_date(strtotime($ntypes['default']), 'custom', NODE_EXPIRE_FORMAT);
}
break;
#1
Hi,
this is my solution is to check if it's necessary to show the expire date field. If it's not required, don't show it.
At line 41 on node_expire.module, change the if condition to this:
if (isset($form['type'])and $form['type']['#value'] .'_node_form' == $form_id
and $ntypes = variable_get('node_expire_ntypes', array())
and $ntypes = $ntypes[$form['type']['#value']]
and !empty($ntypes['required'])) {
To avoid the 1970-01-01 value is as easy as not try to insert an empty default date:
case 'prepare':if (!isset($node->expire) && !empty($ntypes['default'])) {
$node->expire = format_date(strtotime($ntypes['default']), 'custom', NODE_EXPIRE_FORMAT);
}
at line 25 on node_expire.nodeapi.inc.
It works for me
#2
Looking at the nodeapi prepare state, it looks like the default expire time isn't being used right. I changed this to be:
<?phpcase 'prepare':
if (!isset($node->expire)) {
$ntypes = variable_get('node_expire_ntypes', array());
if ($ntypes['default']) {
$node->expire = format_date(strtotime($ntypes['default']), 'custom', NODE_EXPIRE_FORMAT);
}
}
break;
?>
#3
Woops. Patch without the print_r
#4
In the 'prepare' case you should not pull in $ntypes again as it is passed as a parameter. The parameter version already is keyed into the content type of the node whereas variable_get is not.
#5
I made the above change and it worked
however, every time i run cron.php i get the error below
it doesn't appear to break the website, would be good to get rid of message though
anyone any ideas?
user warning: Table 'web33-hartland.cache_views_data' doesn't exist query: DELETE FROM cache_views_data WHERE expire != 0 AND expire < 1245341978 in /var/www/vhosts/coastwise.org/httpdocs/includes/cache.inc on line 166.
user warning: Duplicate entry '50' for key 1 query: INSERT INTO node_expire (nid, expire, expired) VALUES (50, 0, 0) in /var/www/vhosts/coastwise.org/httpdocs/includes/common.inc on line 3422.
#6
@mattys:
I'm not sure about the first warning, but the second one is addressed here (with a working patch): #426636: Errors on cron run
#7
I'm not sure if I should post this here or on http://drupal.org/node/405608
* If I use tororebelde's fix in #1, the default function works properly. However, on a node type that isn't set to require a node expire date, the node expire date field doesn't show up at all, so you can't manually set an expiration for some node.
* If I use the patch submitted on http://drupal.org/node/405608 by arthurf, the field is no longer incorrectly required, but the default expire date and "required" functions stop working. I can set a node type to default-expiration of, say now +60 days, or some specific date, and/or set the expiration to be required, and both are ignored. You see a blank expiration field, and you can save the node without a date entered even if it's set as required in the node type settings.
#8
It seems to me that the correct fix (at least partially) is just to modify line 26 in node_expire.nodeapi.inc to read
if (!isset($node->expire) and is_string($ntypes['default'])) {
That way, the node expire value on a form is not filled with Jan 1 1970 if the default is empty.
Regards,
Nigel
(Edited: Test should be is_string(), not `!= ''`).
#9
Okay. I think I've got it nailed.
There are two changes needed in node_expire.nodeapi.inc.
At line 20, change it to read:
if ($row->expire)
$node->expire = $row->expire;
and at line 26 (27 after the previous change):
if (!isset($node->expire) and is_string($ntypes['default'])) {
Regards,
Nigel
#10
I applied this patch first: http://drupal.org/node/426636#comment-1651286
Then I made the changes suggested by Nigel in #9. I am still getting a "You have to specify a valid date" message and a 1969 date on a node that shouldn't require that node expiration be set (same behavior as before the patches). I may have applied the patches in the wrong order or something. Please advise and thank you for the patches.
#11
The kicker for me was users who couldn't edit the expiration getting "Cannot set node expiration in the past" or whatever since the expiration date was being set behind the scenes as "1970-01-01".
I ended up modifying the OP's code to:
if ($node->expire == '0' && !user_access('edit node expire') ){
unset($node->expire);
}
#12
I'll try this patches, from bottom up. Until then i am using default node expire time without blank, just set some big year or add +999 weeks. This should work for sure until module is perfect :-)
#13
"You have to specify a valid date." from default Expiration Date: 1969-12-31 symptom of this bug, yes?
#14
When will the solution be committed to the module?
#15
eh, think ive just submitted #614098: empty node expiry patch which is similar.
#16
#546494: Disable node_expire completely for certain node types might be of interest to those reading this.