This patch is preliminary but mimics the functionality of the patches to notifications submitted in #371596: Including file attachments in messages. The messaging patches in that issue were committed to messaging-6.x-2.1 and still need work, but it was decided to move the notifications portions to notifications_extra.

Here are the features that still need to be implemented:

  • 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

By the way, someone (Jose?) needs to add an "Attachments" component to this project and update this issue accordingly. We should probably also add a general "Feature request" or "Other" component.

CommentFileSizeAuthor
#8 notifications_extra-499942-8.patch491 bytesdanepowell
notifications_attach.patch1.7 KBdanepowell

Comments

jose reyero’s picture

jose reyero commented
Component: Custom subscriptions » File attachments
Category: feature » task

This looks great as a starting point, and it seems to work, so I've committed this first version, with some changes.
- Renamed to 'notifications_files' (more descriptive IMHO, as files may end up as attachments or download links)
- Added some checks, there may be messages that don't come from notifications.

@Dane, you have commit permissions now for this module.

With that features yet to be implemented, we'll be maintaining and polishing this module here, and then maybe move it to the main Notifications package.

I'm thinking whether it would make sense to have notifications for 'file upload' events too.

danepowell’s picture

danepowell commented

I committed a patch to the DRUPAL-6--2 (dev) branch. It includes the following additions:

  • Adds a settings page allowing users to choose which content types should include attachments in notifications
  • Checks that recipient has 'view uploaded files' permissions before including file attachments
  • Adds tokens listing the URIs of all attachments, separated by either HTML or plaintext line breaks

Jose, I see your point about using a single query for retrieving fids from a list of nids, and file objects from that list of fids. I see that you've already written a query builder function, but I can't tell how to use it for this. Can you document that function or give me some guidance on how to build the queries, or fix the queries yourself? Thanks.

(btw, this is my first commit to CVS, let me know if everything looks okay, or more importantly if it doesn't look okay :) )

jose reyero’s picture

jose reyero commented

Yes, the commit and most of the improvements look good.

I've added a few more checks (like not including files for digested messages), and also I've realized the file objects are already loaded in the node at this stage so no need to reload them.

About the tokens, I think they should be node tokens so they can be actually used in the templates. At this stage, when running message alter, the message should be already composed. Anyway, I'm not sure whether file attachments are already included in the full body by token module...

apt94jesse’s picture

apt94jesse commented

Guys, thanks so much for adding to an already great module. The attachment capability is just what I was looking for. Any chance of you committing that patch to the dev version available on the project page? Since I'm in a Windows environment, it's a pain to auto-apply a .patch file. I manually created the .info and .module files but I get some errors during cron, even though it appears to function as it should.

Again, thanks for your work, this was much needed to add to the notifications system and this seems to do the trick wonderfully.

danepowell’s picture

danepowell commented
Status: Needs work » Fixed

I committed a small change that makes tokens apply to nodes as suggested in #3, and cleans up the formatting of token links.

I think I'll go ahead and mark this as fixed, since I'm not sure what else needs to be done.

As for rolling a new dev release, I'm not really sure how to do that... Jose?

danepowell’s picture

danepowell commented

FYI, I committed a fix for a minor bug that caused some watchdog errors.

estrata’s picture

estrata commented

I am using this with the dev version and it works great (yay).

One problem I noticed (though I may just be being dumb, it happens a lot), is that when there is no attachment, the token [node-files-html] still shows up. When there is one, it is replaced by the URL.

Thought I would mention it.

danepowell’s picture

danepowell commented
StatusFileSize
new notifications_extra-499942-8.patch491 bytes

Thanks for the heads-up. I committed a fix that returns empty strings for node-files tokens if no files are attached to a node. Here is the relevant patch if you don't want to wait for the next dev release.

estrata’s picture

estrata commented

Worked great; thank you!

Status: Fixed » Closed (fixed)

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

danepowell’s picture

danepowell commented
Component: File attachments » CCK notifications

Note that this functionality has been rolled into a plug-in module: http://drupal.org/project/notifications_files

danepowell’s picture

danepowell commented
Component: CCK notifications » Miscellaneous

(accidentally changed component)