Attachments on comments

From reading the description, the comment attachment settings should likely be enabled/disabled on a per-node type basis, just like comments and attachments themselves. Then, you could enable comment attachments for just forum nodes, for instance.

Otherwise, sounds like an impressive list of changes.

NEW patch file attached.
Fixed an error that crept into the insert sql, oops; aso made it more compatible with other modules by not requiring $comment parameter of hook_comment to be optional.

Not likely to get any reviews if you upload zipped patches. Also, 4.6 does not get features. Finally you want to look at http://drupal.org/node/28255 for comment module hooks enhancements.

I could only add one attachment in the original submission, so I zipped the 2 patches. I can post followups with each patch separately if unzipping is too hard :-) Thanks for the pointer. Hopefully with two separate patch contributions for similar functionality, this feature can be added in 4.7.
Boris, I agree with your suggestion; just haven't yet learned how to integrate into content-type settings.. simple module setting was easier.

NEW patch to set comment attachments on/off for each content type.
I had to use node_load to get $node->type (for checking if attachments are on or off for this node type).. if there's a better way, let me know.

Patch for cvs.

oops, forgot 4.6.x needs array('nid' => ..) for node_load.

Let me repeat this: 4.6 does not get features.

Two patches are worse than one for the same. I advise you to postpone this patch until eaton's is commited (with the form API, that is).

Also in upload_comment, you call upload_nodeapi($comment, 'view', NULL).. This is way too hackish.

ok chx, i guess you aren't too fond of users contributing code.. hint taken.
i never expected the 4.6 patch to go into drupal 4.6.x.. that's why i included the cvs patch. the 4.6 patch is for people who want to use this code now (not sure why that needs explaining).

There's a curly bracket missing in updates.inc in the 4.6.3 version of the patch (line 344). Attached a fixed patch. Besides: Your effort is highly appreciated, contrary to what some condescending posts to this thread might suggest.

There are no condescending posts, just non-English as a native language speakers who come off as blunt if you don't know them. Or they had a grumpy day :P

But, as chx points out, this will need to be revisited when the form API stuff hits (soonish, I think). Thanks for keeping up with this.

I applied the patch and I can attach files to comments but the attachments are only visble in the "edit comment" form, not in the normal view of the page. Any idea what could be wrong. Do I have to modify my theme?

I should have read your comments at the beginning in which you state that the theme_comment function needs to be extended in order to render the comment attachments. When I attempted to do that I realized that it would not be trivial because the theme just gets the raw list of files, i.e. an array of file objects. The theme code would need to turn this array of file objects into a <TABLE> of <A> tags. I think this task should be moved to the comment.module. I have modified your patch to do exactly that (borrowing code from upload_nodeapi( 'view', ... ) and it seems to work just fine. Let me know if you are interested in my solution.

Mindless,

I don't think chx was trying to shoot it down -- just noting that major changes (like this one) generally get made to the HEAD version rather than the current (4.6) version. If they're accepted into HEAD, they can be backported to 4.6 etc if there's enough demand.

Since the 4.6 branch is frozen, and HEAD is evolving with new features on a regular basis, it's just safer to code major changes that way.

Also, his other comment about the http://drupal.org/node/28255 patch shouldn't be taken as a slam on your changes. That patch is one I rolled together specifically to make comment enhancements like this one easier to implement. A slightly altered version of it has been committed to HEAD with the formsAPI changes. With the hooks it exposes, adding the 'upload' field to the comment form and displaying the uploaded file in the comment itself don't necessarily require changes to the theme.

So, in a nutshell -- the patch that he pointed you to could consideraly reduce the amount of work (read: code) necessary for this set of functionality. Please don't abandon this! It's just worth noting that major changes like this have to work with HEAD first, THEN go back to a 'stable' version.

After consulting with a couple of other devs, it was determined this should be closed. It's a good feature, but the 4.6 branch is frozen and simply doesn't get new features like this. 4.7 is inches from code freeze and release candidate at the moment, and already has all of the comment hooks that were necessary for this patch.

If you can move this work to HEAD, I'd be 110% behind it.

After consulting with a couple of other devs, it was determined this should be closed. It's a good feature, but the 4.6 branch is frozen and simply doesn't get new features like this. 4.7 is inches from code freeze and release candidate at the moment, and already has all of the comment hooks that were necessary for this patch.

If you can move this work to HEAD, I'd be 110% behind it.

Eaton, all good points, completely agree. We use 4.6.x so of course I had to write code for 4.6.. I posted that patch for others on 4.6 (just got email from someone trying to use it). I also posted this just to give drupal devs some ideas about supporting this feature, in hopes that they/you can integrate similar changes into the evolving code in HEAD.. I'm not necessarily asking for any patch I submit to be integrated verbatim, as I expect drupal devs to know better than me the "right" way to do things. But if changes I request and even provide a reference implementation for can make it into future releases in any form then it will ease our future upgrades. I don't track the latest closely, and realistically you have to expect large sites to use stable releases and make changes to suit their needs.. and then possibly send back those changes, as I've done here.. it is my hope the drupal team can draw from these contributions to make future releases better and better! All that said, if we ever upgrade to 4.7 clearly I or someone will be merging all our patches at some point.. if you think there is a decent chance of this feature making it into 4.7 if I can produce an updated patch in the next day or so, then it would be worth my effort to make that merge now.. otherwise I will wait until final 4.7 release to merge against the actual-final code and not do it twice.. thanks for reading :-)

hi, which is the latest version? Also, does the latest version require hacking to display attachments? (Im referring to Hanu's post)

Thanks.

V

yes, that's the latest. if your theme doesn't override theme_comment then you don't need any additional change.. if your theme does override this then you need something like this added:
print implode('', module_invoke_all('comment', 'view', $comment));

Im probably missing something obvious but where do we turn attachment on comments on? (I use 4.6.2)
admin/settings/upload gets its stuff from :

function upload_admin() {
system_settings_save();

}

your patch puts the radio button here:

function upload_nodeapi(&$node, $op, $arg) {
switch ($op) {
case 'settings':
$form = form_radios(t('Attachments'), 'upload_'. $node->type, variable_get('upload_'. $node->type, 1), array(t('Disabled'), t('Enabled')));
if (module_exist('comment')) {
$form .= form_radios(t('Attachments on comments'), 'upload_'. $node->type .'_comment', variable_get('upload_'. $node->type .'_comment', 0), array(0=\
> t('Disabled'), 1 => t('Enabled')));
}
return $form;

okay, its on the content type settings page. I think the readme shouldnt say upload settings page

Is this patch likely to make it into 4.7?

is there a forms API updated version?

how about a HEAD version, mindless? once this gets in, it will be easier to rip out 'follow-ups' from project.module and just use comments instead.

I guess my comment #17 was too long.. no one is answering my question.

(which is basically comment #22.. if I bring this patch up to date now, what are its chances of being in 4.7?)

IMO, it is too late in the 4.7 release cycle to accept such a change. But we could accept it as soon as the HEAD branch reopens will be soon thereafter.

But... Given the fact that the comment form/view/etc hooks now exist in HEAD, would it be possible to provide the functionality as a pure module? I know that chx had a rough 'comment upload' module working when the comment_hooks patch was being tested, and it didn't require any additional patches.

Thoughts?

If I find the time (or someone else does...) to update my comment_upload, it could live as an example module (at drupaldocs), it's so simple. It lets you only upload one file, but that's more in line with comment anyways.

I found too many bugs in normal node attachments in current cvs anyway.. doesn't seem worth trying to make this patch work yet.

Hanu,

can you share your modification to comment.module to display files?(theme_comment_view Im assuming).

Im still not clear on the modification I need to make to themes but I would rather not go down that road considering I have many themes.

This is the line that does it in comment.module, function theme_comment:
$output .= implode('', module_invoke_all('comment', 'view', $comment));

In our phptemplate based theme we use:

Will attachments on comments be part of 4.7?

The infrastructure to support them is there, and there's an example module out right now that demonstrates the functionality (http://drupal.org/node/37197)

I'd imagine that it will have to be polished up a bit before it's ready for production, but the answer is 'yes.' With a small plugin module, and NO core patches, 4.7 will support attachments to comments.

Great! Thank you!

If folks would help me code betterupload, then this issue is tackled too. betterupload has comment attachements "for free".

Sorry if this is OT, but it seems that we're able to attach files to comment. This is exactly the functionality I'm looking for. A quick pointer is appreciated!

Oops changed title by accident.

The project module currently has its own node type for issues, and replies to them. It's not portable to other nodes, and the project module is not really portable to anything but drupal.org right now, either. :-) A rewrite/upgrade is in progress and I understand expected to be ready very soon, if it's not already. I'm looking forward to it myself.

Just marking this as 'won't fix'. The FormAPI changes in 4.7 make the patches to core unecessary -- a contrib module can now use the form_alter hook to alter the comment form, and hook_comment's 'view' op to display the downloadable file.