GHOP #51: Port Comment Upload module to Drupal 6

Thanks for working on this. What were you doing when you expected the hook to be called? Also, have you checked whether comment_invoke_comment is called and whether module_implements('comment') includes your module?

Btw, Heine (the module maintainer) says that a better target rather than HEAD is the 5.x-dev version of the module.

I'm marking this needs code needs work to hopefully get some eyes on it.

hook_comment is no longer invoked on display of the comment form as the following code was removed in http://drupal.org/node/82526.

  // Graft in extra form additions
  $form = array_merge($form, comment_invoke_comment($form, 'form'));

This needs to be documented in the update docs and the hook_comment API docs need to be updated.

Webchick told me I should answer this better. If you want to modify the comment form on display, you need to use hook_form_alter.

Working on it now.
I'll not submit any code yet as I'll still need to solve some problems, when I have some decent patch (probably tomorrow) I'll upload it. The problem isn't as simple as moving the hook_comment form part to hook_form_alter, there are some upload functions not present anymore in Drupal 6 and I'll probably have to change some of the structure of the module so it'll work properly with the new functions.

I need some help with setting passing the $comment->files variable so comment_hook can detect it. In the present way the code work I only have access to the form variables and these are gone after the form is submitted. Is there any hook or something I could use to change the variables passed to update and/or prepare?

I don't know if what I said makes any sense but I've attached what I've done so far in case anyone can take a look.

(the docs on converting 5.x to 6.x modules need some serious upgrading BTW, there is nothing about several things I had/will have to change)

The best way to proceed is probably to study node.pages.inc and its interaction with upload. What you need is the files in $form_state['values'].

I imagine this can be done by unsetting the standard #submit for the form (comment_form_submit) in comment_upload form_alter, then make button specific submit handlers that point to a comment_upload function (eg $form['submit']['#submit'] = array('comment_upload_form_submit');), but keep upload_node_form as a form submit handler. ($form['#submit'][] = 'upload_node_form_submit';).

Now, in comment_upload_form_submit, emulate node_form_submit_build_node, eg:

function comment_upload_form_submit_build_comment($form, &$form_state) {
  // Unset any button-level handlers, execute all the form-level submit
  // functions to process the form values into an updated comment.
  unset($form_state['submit_handlers']);
  form_execute_handlers('submit', $form, $form_state);
  _comment_form_submit($form_state['values']);
  $form_state['rebuild'] = TRUE;
}

Don't forget to change the key ['new']['attach']['#submit'] as retured by _upload_form from node_form_submit_build_node to comment_upload_form_submit_build_comment.

If you need help, just post in the issue.

Hmm, sorry about the delay, was working on the code.
I think I've got most stuff to work.
There are only two problems that my updates currently seem to cause, comment previews aren't working and if you click on Upload and don't actually submit the comment and after that create another comment, the previously uploaded file will still be there.

Feedback on the current state of my code and help on the first bug would be appreciated (I think I know the cause of the second, but it's too late here for I to solve it today).

EDIT:
Forgot to say that I also added a weight feature to keep Drupal's UI consistent, but it's not working and I don't know why.
The patches are against version 5.x-1.x-dev.

The patch applies smoothly, adding attachments work until i hit preview. On preview the attachments list empties, and only gets the previously uploaded files when I upload a new one. I suspect this is due to a faulty form rebuild in the preview function. What is weirder is that when i start submitting a comment, I add attachments and then I abandon the whole process, then when adding a comment again after adding one new file the list will show all the previously uploaded ones.

With the default drupal 6 setup I can't even submit a comment, because it keeps dropping me to preview. I changed the comment settings not to require preview, it works then fine.

So overall it does what it should, but there is a glitch still in the workflow. I would poke around the form rebuild functions.

I need some help fixing an issue with comment_upload, I cannot find out why preview isn't working. If I comment the $form['#ahah']['path'] definition in upload.module, everything works fine. However if I leave it uncommented and comment all the things seemingly called due to it (comment_upload_menu, comment_upload_js) I still cannot preview any comment.

Does anybody know the reason of this issue?

Any aha element sets #cache to TRUE. This means that the form will be fetched from cache, and not rebuild via the form builder function (comment_form in this case). Adding the #after_build function that generates the preview is dependent on $_POST['op'] being 'preview' in comment_form() (yes, a suboptimal design). So, the when the form is build, there's no #after_build to generate the preview. Upon submission of the form via the button Preview, the form builder is not called because the ahah element caused it to be cached.

Possible solution: Create a comment_upload preview handler that generates the preview in the same way as node module does, bypassing comment module.

A note on the patch:

- please use two spaces for indentation, not tabs.
- how do you create the patches? You may want to try using cvs diff -up ; this would create a patch with proper patch headers and would include all modifications to the three files in one patch.

Etinin, thank you for your persistence. I'm inclined to triage the preview behaviour as a comment module bug. Further work on preview probably requires this to be solved.

I filed a bug against comment.module http://drupal.org/node/203417

@Heine: would it be possible to allow the student to finish this task before the bug you filed against comment.module is fixed? The patch probably wouldn't be marked RTBC but if it was otherwise ready to go and you were OK with it I think we could give the student credit so that this task doesn't prevent him from working on another GHOP task while the comment.module bug works itself through the system.

Sorry about my previous method of creating patches, I was using 'diff -u' manually, I've created a patch of my updated version with the CVS diff method. About the identation, I guess Emacs wasn't doing its job properly, I fixed all of the identation issues I could find manually.

I've attached the new version of my patch, the only two issues left are the weight I couldn't get to work and the preview stuff. If aclight's proposal is possible, even if you mark the GHOP task as done, I can continue working on this after the preview issue is resolved (I've been working on a solution for that myself, when I get some working code, I'll upload it to the issue).

It looks like you're not actually using CVS for your code, right? Instead you're comparing your patched code to a clean version:

diff -urp comment_upload/comment_upload.info comment_upload_new/comment_upload.info
--- comment_upload/comment_upload.info	2007-06-18 19:53:34.000000000 -0300
+++ comment_upload_new/comment_upload.info	2007-12-25 04:04:45.000000000 -0200

That's totally fine, but what I meant earlier was that if you checkout a copy in CVS, you can modify that copy directly. When you use cvs diff to create the patch, CVS will make the patch based on the CVS version, so there's no need to have two separate copies of the code base (the modified and the clean version).

There is also this bit at the end of the patch that shouldn't be there:

diff -urp comment_upload/CVS/Entries.Log comment_upload_new/CVS/Entries.Log
--- comment_upload/CVS/Entries.Log	2007-12-25 04:10:04.000000000 -0200
+++ comment_upload_new/CVS/Entries.Log	2007-12-25 04:10:04.000000000 -0200
@@ -1 +1,2 @@
 A D/po////
+A D/contributions////

I'm not sure what would cause such a thing to creep into the patch.

In any case, this is just a minor problem with the patch, but I don't have the time right now to look at it in more depth.

Sorry, I think I used the wrong command again, meh, I'll create a patch properly with cvs diff -up as soon as I can, unfortunately I'm busy now.
But I'm quite sure I compared my code to a fresh CVS copy, but I think I used a different CVS command.

Etinin, nothing to be sorry about, using CVS is just easier for both of us. Marking finished is fine, if this is possible under the ghop rules.

Yep, chx closed this GHOP task officially, so the student has gotten credit for the work.

Thanks for closing the GHOP issue, at least I can now work on something else while the preview bug isn't fixed.
I've attached a proper version of the patch created with cvs diff -up.

Hello Everyone,

edit: updated regressions section to list all known regressions

Figures, I just found this thread after spending a day to upgrade comment_upload to Drupal 6.x.

Here's my code. It's rather heavily changed because I wasn't planning with other people in mind. However, since it seems there's some interest in having this module ported I might as well post it.

This code really needs to be looked over, but it works for me.

The patch was based off of 'comment_upload-5.x-0.1.tar.gz'.

Features:

* Revised schema to use Drupal's internal file handling system.

Regressions, I polished the features I needed (it was a last minute decision to post this patch here):

* Some of the display aspects aren't working correctly, themes haven't been updated.
* There are a few hacks to get around some differences between Drupal 5.x and 6.x (most notably the missing 'form' $op in hook_comment).
* Single attachment mode is probably broken as I didn't spend time upgrading that feature (sorry).
* Non-javascript (ie: non AHAH) interface was not tested or upgraded, it's probably broken (sorry)

Useful to anyone?

James.

Single attachment isn't working for me either on James patch (as stated #27) and last Etinin patch (#25).

I checked the Etinin patch and seems to me it lacks of either #attributes multipart and #submit:

On the function comment_upload_form_alter I would add them:

    if (variable_get('comment_upload_single', 0)) {
      $form['upload'] = array(
        '#type' => 'file',
        '#title' => t('Attachment'),
        '#size' => 16,
        '#description' => !empty($cobj->files) ? t('You already have a file uploaded, if you upload another it will overwrite the current one.') :''
      );
      $form['#attributes'] = array('enctype' => "multipart/form-data");
      $form['#submit'][] = 'comment_upload_form_submit_build_comment'; 
    } else {

I can't upload you a patch right now because I'm still working on the module and I have a lot of debug around.

Would be possible to have a release of the module for the 6.x?
I had problems to apply both the patch offered (did it manually) so would be a great help for all us, I guess, to have a Development snapshots uploaded or a CVS tagged as DRUPAL-6

Best.

ema

Subscribing. Patch doesn't apply to dev versions anymore. Why not release a 6.x-dev branch?

Just to let you know if someone is interested in testing an alternative approach: There is a patch to add support for webfm attachments for comments. Preview also works. You can find the patch here: http://drupal.org/node/283304

Up

Hello Everyone,
Created a module comment_upload for version Drupal 6.x.
This is not development of old version 5,it is a new module, that does not imply upgrade of the old version.
Not supported Single mode and Inline mode.
Everything else works.
Sorry for my English :)

A new version of this module without an upgrade path is not appropriate for this issue, which is to *upgrade* the module from D5 to D6. Given that drupal.org itself uses the comment_upload module, a "port" with no upgrade path is not going to be very useful in a lot of ways.

thanks for posting that, Helg, even though it doesn't address the upgrade from 5.x issue. I have a site built in 6.x and desperately need file upload for comments, so I'll reviewing this.

Thanks.

I will branch 6.x-dev today, as this is holding up the d.o. upgrade to 6.x

:(

I will branch 6.x-dev & commit when the CVS issues (http://drupal.org/node/289691) are solved.

A 6.x branch exists with a development snapshot on http://drupal.org/node/292582 . Sadly, I've applied none of the 'patches' in this issue.

NOTE: "This development snapshot is intended for testing and development. It is therefore not recommended nor supported for use on websites."