Thanks for this module!

Is this really necessary?
&& $row['status'] == 1

CommentFileSizeAuthor
#6 remove_obsolete_code-853976-6.patch790 bytesjstoller
#4 old-nodes-dates-853976-04.patch1.01 KBsoulfroys
#2 853976-default-published-at.patch770 bytesdgtlmoon
#1 publication_date_date_1969.patch497 bytessoulfroys

Comments

soulfroys’s picture

soulfroys
Primary language Portuguese, Brazil
commented
StatusFileSize
new publication_date_date_1969.patch497 bytes

Is this path ok?

dgtlmoon’s picture

dgtlmoon commented
StatusFileSize
new 853976-default-published-at.patch770 bytes 
function publication_date_nodeapi(&$node, $op, $a3 = NULL, $a4 = NULL) {
  switch ($op) {
    case 'load':
      $node->published_at = _publication_date_get_date($node->nid);

      // we have to manage the 'old nodes', i.e nodes that have been published BEFORE the activation
      // of this module.
      if (!$node->published_at) {
        $row = db_fetch_array(db_query("SELECT created, status FROM {node} WHERE nid = %d", $node->nid));
        if ($row && $row['status'] == 1) {
          $node->published_at = $row['created'];
        }
      }
      break;

I think we still need the && $row['status'] == 1 as this means $node->published_at is only set if the node is published.

maybe it can be simplified as the following, saving the SQL query?

    case 'load':
      $node->published_at = _publication_date_get_date($node->nid);

      // we have to manage the 'old nodes', i.e nodes that have been published BEFORE the activation
      // of this module.
      if (!$node->published_at && $node->status == 1 ) {
        $node->published_at = $node->created;
      }
      break;

what do you think? see patch..

dgtlmoon’s picture

dgtlmoon commented
Status: Active » Needs review
soulfroys’s picture

soulfroys
Primary language Portuguese, Brazil
commented
Version: 6.x-1.1 » 6.x-1.x-dev
Priority: Normal » Critical
StatusFileSize
new old-nodes-dates-853976-04.patch1.01 KB

Sorry for the long-delayed response... :(

What do you think about this?

       // We have to manage the 'old nodes', ie, nodes created That Have Been
       // (Published or not) BEFORE the activation of this module.
       if (!$node->published_at) {
         $node->published_at = $node->created;
       }
samerali’s picture

samerali commented

#2 number makes sense to me.

I see why #4 soulfroys wants to take away status=1 (i'm on the same position) but this will affect on data integrity.

Your custom code should handle if ->published_at is empty then use ->created, @ templates and/or views.

jstoller’s picture

jstoller
he/him
commented
Title: 12/31/1969 for unpublish nodes date » Obsolete code in hook_nodeapi
Priority: Critical » Normal
Issue summary: View changes
Related issues: +#2355633: Obsolete code in hook_node_load
StatusFileSize
new remove_obsolete_code-853976-6.patch790 bytes

Just cleaning up the issue queue and couldn't leave this alone.

As noted in #2355633: Obsolete code in hook_node_load for the D7 branch, that entire chunk of code was made obsolete by the install function added in #561754: Empty pub dates break view sort order. The attached patch removes it.

  • jstoller committed 00714c3 on 6.x-1.x 
    #853976 by jstoller: Remove obsolete code in hook_nodeapi().

We don't...
jstoller’s picture

jstoller
he/him
commented
Status: Needs review » Fixed

I don't have any plans to create another release for D6, but I've committed this to dev, just to get the bug off the books.

Just in case it comes up, I agree with @sameronline in #5. Replacing published_at with created when it's empty is a decision that should be made by a site's developer and not decreed by this module. Adding that capability would be a significant new feature and not one I personally intend to develop.

Status: Fixed » Closed (fixed)

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