Upgrade Attachment to 5.x

if people want to provide them, i'll review and commit patches.

btw, if I have a drupal website with attachment module, and then I upgraded my drupal version to 5.0, as there is no 5.0 version of attachment, what happens to my old files attached to nodes? They are still here, but i cannot access them anymore...

Well the project page has been pretty clear:

UPDATED Sept 13th, 2006

I just took over as maintainer for this module. I've been looking at committed most of the outstanding patches and creating a 4.7 branch. I'm not planning on updating this for 5.0 but I'll review and commit patches if anyone is willing to provide them.

I know... something I'm trying to figure out is how to modify existing nodes to use core "upload" module instead of attachment then...

I think many of Drupaler want this module updated, and this will be better if the module have Ajax update...
please update...

I have successfully made the module post new attachments to a node.

The trouble I'm having now is displaying the attachments for a node.

Here is the 4.7 code

    case 'view':
      foreach ((array)$node->attachments as $attachment) {
        if ($attachment['working']) {
          $file = module_invoke('filemanager', 'get_file_info', $attachment['fid']);
          $workingurl = str_replace('&', '&', module_invoke('filemanager', 'url', $file, TRUE));
          $activeurl = str_replace('&', '&', module_invoke('filemanager', 'url', $file, FALSE));
          $node->body = str_replace($activeurl, $workingurl, $node->body);
          $node->teaser = str_replace($activeurl, $workingurl, $node->teaser);
        }
      }

      // If this is not a teaser add our attachment list to the end of the body
      if (_attachment_countvisible($node)>0) {
        if (!$arg) {
          $node->body .= '<br /><a name="attachments"></a>' . theme('attachments', $node);
        }
        if (variable_get('attachment_display_teaser', 0)) {
          $node->teaser .= '<br /><a name="attachments"></a>' . theme('attachments', $node);
        }
      }

I understand there have been changes to the hook_nodeapi, but I don't understand how to implement them.

Here is the proposed 5.0 code:

    case 'view':
	foreach ((array)$node->attachments as $attachment) {
      if ($attachment['working']) {
	      $file = module_invoke('filemanager', 'get_file_info', $attachment['fid']);
          $workingurl = str_replace('&', '&', module_invoke('filemanager', 'url', $file, TRUE));
          $activeurl = str_replace('&', '&', module_invoke('filemanager', 'url', $file, FALSE));
        // Add the attachments list to node body with a heavy
        // weight to ensure they're below other elements
          if (!$teaser) {
            $node->content['attachments'] = array(
              '#value' => theme('attachments', $node->attachments),
              '#weight' => 50,
            );
          
        }
      }
	}

Can somebody tell me where I'm going wrong?

Attached is my progress.

Well, I was mistaken.

I am not successfully posting new attachments. However, I am successfully displaying old attachments.

Attached is a stripped version of the module whose purpose is just to display old attachments (backward compatibility). I suggest you use this for old nodes, and enable upload.module for new ones.

I too would like to see this module upgraded to support Drupal 5.x. I run several Drupal websites now that depend on this module to store files, and I would like to be able to upgrade those sites someday. :-)

Thanks,

-- Doug

How about a script moving all the old files from the attachment module dataset to the regular upload module. It would mainly involve a db work. This way, the attachment module wouldn't required anymore.

Here is a patch to port attachment to 5.1. I already handled filemanager, so you'll have to go get the patch from http://drupal.org/node/99016 The two modules now work well for me.

same complaints on the .install file as in #99016. also if you're going to offer a hook_deinstall() implementation you should delete variables too.

in the .module overall things look good. mostly style things

in attachment_form_alter() what's the purpose of the $old_type variable?

i think that:

        // We default to a collapsed region, but it's annoying if it's closed when we actually have
        // something attached.  Test for it and set accordingly.
        $collapsed = true;  // Default to collapsed.
        if( isset($node->attachments) )
        {
          $collapsed = false;
        }

could be simplified to:

   $collapsed = !isset($node->attachments);

i like to put the hook_menu implementations toward the top of the file. it makes it easy to see what urls the module provides and what functions do it. kind of like a little table of contents. that's just a personal preference.

review the drupal coding standards for:
- array declarations in attachment_nodeapi($op='view') and attachment_link()
- function declarations in attachment_nodeapi($op='prepare')

why are you using strcmp() in theme_attachments()?

so plenty of little fixes but like i said things seem good.

drewish,

Ok, I think I've got it per Drupal coding standards, but feel free to comment. :) One specific item I didn't understand was: "- function declarations in attachment_nodeapi($op='prepare')" Not sure what the problem there was.

For your specific questions:
* .install file: I hate spurious error messages. It causes the end user to go looking for a solution to a problem that doesn't exist or causes insecurity about the module. Hence testing if db_table_exists(). Besides, I assume 5.0 added that function for a reason; might as well use it. However, the rest of your comments are corrected for: changed some minor formatting and added the calls to delete the variables.

* "in attachment_form_alter() what's the purpose of the $old_type variable?"
For some reason "case 'node_type_form' needed it to work. This is per item #20 in the "Converting 4.7.x modules to 5.x" document. http://drupal.org/node/64279#node-type-settings

* "i like to put the hook_menu implementations toward the top of the file."
Done and good point.

* "why are you using strcmp() in theme_attachments()?"
This is to avoid something ugly that annoyed me. The title field defaults to the filename text. So without it you get the link in the node looking like "filename (filename)" if the user didn't see fit to change the title.

For your convenience per your comment in the filemanager patch, I added myself to the CREDITS file. :)

The new patch is stand-alone against the orig sources and shouldn't be applied over my previous patch.

This has now been added to CVS and a new 5.x Dev release created. Please update your sites and let me know if you have any problems. If I haven't heard of any major issues with this update, I'll produce an official release in a few weeks.