Including file attachments in messages

Subscribing

subscribing

Right, there's no support yet for this in Messaging.

However, we are not adding any new feature for 5.x, so maybe for 6.x (if someone provides the patch)...

I'm considering this not a "minor" feature request as I'd like to be able to send file attachments via both e-mail and MMS, and perhaps other methods.

I believe this should be relatively easy to accomplish at the level of individual Messaging modules. For instance, with the PHPMailer plugin one would merely need to check if the node has attachments, get their file paths, and then use the PHPMailer class function AddAttachment to add them to the outgoing message.

Actually to do this properly might be somewhat more complicated, but it might be easy for someone who is more familiar with the code. I believe you would need to modify the Notifications module (probably the notifications_process_message function) to add any attachments to the $message object. Then, you could do as I describe above and simply pull them back out in the PHPMailer module and use the AddAttachment function to actually add them to the outgoing email.

Okay, I am including a VERY preliminary patch against 6.x-1.1. If a node has one or more attachments, and the sending method is PHPMailer, this patch will include the first attachment with the email.

Really I am looking for feedback and assistance here rather than encouraging anyone to use these patches in production - I have no idea how robust they are, all I can say is that they work for me. This is my first time doing any serious coding for Drupal :) Obviously there needs to be lots of improvements- some sort of option letting admins/users choose whether to include attachments, compatibility with other Messaging submodules, and the ability to handle multiple attachments.

You will need to apply the patches to the Notifications and Messaging modules, respectively.

Here is an updated patch against 6.x-1.1. Multiple attachments can now be handled.

Again, this is working fine for me, but it probably needs a lot of work before it is ready for production.

Cheers!

@danep: there have been a lot of changes in messaging.module so I don't think your patch can be applied to a CVS checkout. You'll need to roll a patch against the latest DRUPAL-6--1 branch (although there's also a DRUPAL-6--2 branch...)

Here are the updated patches against 6.x-2.0-beta1

This starts looking good, though I don't think it will be ready yet for this next version (will be 6.x-2.0 in a few days)

Anyway, if we get some messaging api ready, we could add this into messaging. The Notifications side needs a lot of things yet.

Some things I'm missing here, for the messaging part:
- Handling more than one attachment.
- Handling queueing (messaging store save and load with attachments, we can use the serialized fields for that)
- What happens with the methods that don't support attachments, should we add them as a download link? Make an option? Add some admin notice like 'This method supports attachments'?

From the Notifications side, this small patch is ok for testing. But for having something usable, we need to add some options (like whether to include attachments or not, ideally per content type), or whether to actually attach the files or having them as a link in the mail (tokens).

So I suggest we first get the messaging part working then continue working on that on the Notifications side which will be some more work.

Changed title back.

@just-work,
The problem is that if we add this with no option for including / not including the attachments everybody will get these huge emails when there are files attached to nodes, and this may sink performance for some cases or sites.

Anyway, let's focus in the messaging part, the notifications part will come later.

On the Messaging side:

• The most recent patch that I posted should handle multiple attachments
• Since queuing doesn't appear to specifically invoke any message components, and attachments are stored in the message object as a text file path, I see no reason why queuing shouldn't work with the current patch.
• I think that as we add support for attachments to sending methods, we should add notices like "This method supports attachments", and have a checkbox for "Include attachments with messages sent via this method." For all methods (including those that don't support attachments), I think tokens should be available to add download links using the Message Templates feature.

For Notifications, I think we just need to add a checkbox for each content type saying "Include attachments with notifications of this content type." Thus, attachments would not be included in messages unless (1) attachments were specifically enabled for that content type in Notifications, and (2) attachments are specifically enabled for that sending method in Messaging.

Later, perhaps we can add options like "Don't include attachments larger than x bytes", etc...

seems like it might be ideal to make the attachments an array of file objects, given that files become first class objects in drupal 7.

Here is an updated patch for notifications against beta5. This includes a toggle to include attachments at admin/messaging/notifications.

I'll put similar toggles in the Messaging module and try to get tokens implemented.

Here is an updated patch for Messaging against beta5. This adds a toggle for including attachments to the PHPMailer sending method.

Jose, in trying to implement a token, I discovered that tokens no longer seem to be available in Message Templates. I'm sure that they used to be available, so is this a new bug?

Jose, I am trying to get attachment tokens implemented but I need a little bit of guidance, as I haven't dealt with Tokens before... can you at least give me an idea of where to start?

Also, if someone would test the latest patches I think that they could be rolled into the next release as-is, and we can add additional functionality later.

@danep: what do you think about my comment at #18, re: $message->attachments as an array of file objects? This would allow us to provide other useful parameters like filemime in addition to filepath.

@mfb: Your idea sounds interesting, but I am not familiar with "first class objects" and don't know enough about file handling in Drupal to say whether this would be a "good idea" or not. I certainly don't know enough to implement it myself :)

It's not much to implement. I'm just saying the way we add attachments to a message would be:
message->attachments[] = $file;
where $file consists of, at a minimum, $file->filepath and $file->filemime.

then in your phpmailer patch you'd have something like

  foreach($message['attachments'] as $file) {
    $mail->AddAttachment($file->filepath);
  } 

And as far as other messaging methods, I believe mimemail actually requires a filepath and filemime, not just filepath.

Ah, I see- I thought you meant somehow embedding the actual file in the array. It should be easy enough to add MIME information as you suggest, I'll investigate that.

One concern I have is how message queuing could affect things. For instance, what if someone posts content with an attachment, and then changes the attachment? If the attachment is changed before the message is actually sent, the MIME type may not match. The filepath may not even exist anymore- I am not sure what kind of behavior this would produce, i.e. whether it would just fail silently or do something unpredictable...

Should we consider moving more of the logic to the Messaging module, so that attachments are actually retrieved at the point and time that the message is sent?

I was actually thinking there could be some use case for attaching a file which has not been stored in the file system, which would require passing the actual file data in the array. But it's not something I need so... :)

If a file disappeared before the message was sent, then yes the message would be missing an attachment. But to avoid this we'd have to do something like copy every file in a message to a new temporary file, which seems like overkill to me.

I agree that creating a separate file store for Messaging is overkill. What if instead of Notifications looking up the attachment and sending the file path to Messaging, Notifications just sent the node ID, then Messaging uses that node ID to retrieve attachments?

I could imagine different use cases. Some sites might want to build the message based on the version of the node as it existed when the message was queued, not the current version when the message is finally sent. In this case if you build the message later you'd have to use the node vid -- and hope revisions are available so it's even possible to get an old version of the node.

If you ask me this is being over-thought, and we should just queue up the message for delivery; too bad if the node is later edited or deleted before the message is sent, there will be some stale data or missing files. But, I am new to Messaging module so it's just MHO.

I agree, I think it's most important that this simply gets rolled into the module; we can worry about details later after some user feedback starts rolling in.

Here are updated patches against beta6. These include handling for MIME type information as well as the given file name (not just the literal file path and name). For some reason the attachments still show up with the literal file name instead of the given name, but perhaps that is just a quirk / security feature of Gmail, my only mail reader.

The messaging patch could also include support for attachments in messaging_mime_mail.module:

return mimemail($mail['from'], $mail['to'], $mail['subject'], $mail['body'], NULL, $mail['headers'], NULL, $mail['attachments'], '');

you have $attach_object as an array, but you should just make them objects, especially if you're calling it $attach_object :)

Haha... good catch mfb. I am curious (I am not very versed in PHP), is there an advantage to using objects over arrays?

Anyway, this patch makes the variable names a little more appropriate. It also includes the modification that you proposed for MIME mail. However, I cannot actually test MIME mail functionality. So while I have every reason to believe it would work, I can't guarantee that it does.

Objects and arrays are similar in PHP, with each having some advantages.. Drupal core has file objects so that seems best in this case.

so you would need $mail->AddAttachment($attachment->filepath, $attachment->filename, 'base64', $attachment->filemime);

I'm already using the mimemail part of this patch to send some e-mails with attachments via messaging module to my cellphone's MMS gateway and it seems to be working great.

Glad to hear it! Now that I think about it, it probably makes sense in this case to have an array of objects (each an individual attachment). I updated the code to reflect this.

Is there any more work that needs to be done on this before it gets reviewed and rolled into the next release?

Hey,

Some thoughts. This is going to be a fairly sizable addition so Jose will have to look at it as well. This could be pretty cool though.

1) Ideally don't make haphazard phpmailer settings for attachments--we already have mimemailer that doesn't heed them. On the messaging side of things it looks like the most obvious place to put this would be /admin/messaging/settings/method and have a setting per send method alongside "message body filter" called like "attachment handling" and saying like "None, One, Multiple" or something. Types that didn't support this would be blank or provide none or something. I haven't looked into doing this, it would probably require some additional method information to be passed to messaging and a few patches to that form area.

2) We probably want some way of configuring attachments per message_type as well. The best way I see would be to go through the messaging_parts in the hook_messaging already available in modules like notifications_content, for instance. I think we could create and use a semi-reserve part, "attachments." It would work as follows, with the example of notifications_content:

notifications_content has a few messaging templates for node events with subject, content, and digest line. We add the attachments field which we can say will always be a comma-separated list of file ids. We default it to something like '[node-files]', which is a token notifications_content can define. PS, as Jose said before, ideally you should also expose a token like [node-attachment-links] that could be used anywhere in the body of the message and would have similar logic to [node-files] in this case but would output a list of links.

messaging shows the 'attachments' field on the parts admin pages as usual. The default has the token, if it is overridden with <none> that will effectively mean no attachments.

Similar to your change in notifications_process_message(), make it get the $attachments the same way it gets the other notifications parts, explode it and parse the array properly, and send it off to messaging as it does now.

See if that workflow makes sense and if you can get it done. I think especially #2 is actually not at all difficult to code, and the first is just a matter of messing with some forms. Some of the current patch code could carry over too.

Thanks,
David

Sorry, status change.

David

After talking about this, David and I have decided we are going to try to speed up this issue, as this is really a nice feature and a lot of good work has been done here. So I've committed the messaging part of the patch with some minor changes:

- The message object may have a files array, indexed by fid. They will be converted into $mail['attachments'] when sending mails, see messaging_mail_prepare.
- Added serialization/retrieval for the messaging store.

The idea in short, is this one: You can add $message->files, but then it will be up to each sending method what to do with it. I.e. mail methods may send attachments, but sms or xmpp methods my replace it by a (maybe shortened) download link.
Thus, any settings here will be per sending method.

Note that the current Notifications part will be broken since $message->attachments is now $message->files. About this, the Notifications side of things, it will take a bit longer, we really need more options here and the current patch though it works is a bit rough yet. Ideally we'd like to have:
- Per content type settings
- Some tokens to optionally have attachments as links in the message body.
- Check visibility permissions, as the attached files may not be visible for users

We think the best way to move forward with Notifications/attachments is to create a new notifications_attach, or notification_files module which ideally would be in this "Add ons" package at least for now, http://drupal.org/project/notifications_extra

This will allow us moving faster with that one, while we try to keep the Notifications package as stable as possible. So:

@Dane, if you think this is a good idea you are welcomed to become a maintainer of that notifications_extra package so you can develop and maintain this attachments module in there.

I think all this can be implemented by using the message_alter() hook, but if you need anything else, let us know, we can add more hooks if needed...

I think that sounds like a great plan. I would be happy to be a maintainer for Notifications Add-ons. Let me know what I need to do to make that happen, and I'll start polishing things up.

To clarify re: The message object may have a files array, indexed by fid: So far I have been using $message->files[] = $file; but sounds like I should be doing $message->files[$file->fid] = $file;

@mfb,
Right. that is to allow easier serialization/storage/retrieval and to prevent duplicate files from being attached.

@Dane,
The first thing you need is a cvs account, follow the instructions here, http://drupal.org/node/59
"If you are applying for a CVS account because another module maintainer has agreed for you to become a co-maintainer of their module...."

Let's continue working on the messaging side of things in this thread, and the notifications portion at #499942: Include file attachments in messages, where I have "ported" the notifications side of the patch to notifications_extra using hook_message_alter().

Thanks, looks like we are really making progress.

So now messaging supports file attachments (As of 6.x-2.1), still it needs the notifications part we'll be working on in notifications_extra

I'm closing this one, we can create new issues for other sending methods in this project (there's some other patch around for adding MMS support, that one could use files too).

Thanks everybody for the great work here.