Here's the start of a patch to port this module to 6.x. It at least enables now, and shouldn't display a fatal error upon doing so. ;)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick’s picture

patch.

webchick’s picture

Here's another iteration. Gets rid of a fatal error about operands on comment submission.

I'm currently blocked on drupal_mail changes in 6.x, which my brain is not parsing very well at this hour.

sun’s picture

Didn't look at the patch. Just want to leave a note/reminder about http://drupal.org/node/50827

add1sun’s picture

Just a few more cleanups but not tackling actually making mail work yet. ;-)

add1sun’s picture

OK, this isn't pretty, but mailing now works. The new commentmail_mail function needs to be cleaned up a bit and the deleteban link in the email was telling me the comment didn't exist anymore (when it definitely did) but I didn't have time to even look at that. All the other links in the email were fine. I have not tested this much but I'll try to do a code cleanup/more thorough testing later tonight, though that may not realistically happen.

Basically I think this is close and just needs a touch of love to bring it home.

wwwoliondorcom’s picture

Any update ?

And do you know any other module which can do as comment mail ?

Thanks.

bonaparte’s picture

Thanks for the patch. The mail definitely works. I attempted to fix deleteban.

Disclaimer: I'm not a sophisticated Drupal developer. I'm still learning Drupal development. It is exciting to get this module ported to Drupal 6.

Is $cid supposed to be an integer (the comment id)? I found out that it is rather an array in the function commentmail_deleteban($cid).

I'm ready to help port this module to Drupal 6. Is there anything I can do to assist you folks? I can spend an hour or two everyday for this task.

bonaparte’s picture

@add1sun

The previous comment is intended to be a reply to add1sun's latest patch.

bonaparte’s picture

commentmail-6.x-188698-5.patch needs to be re-rolled

bonaparte’s picture

FileSize
10.76 KB

I manually patched commentmail-6.x-188698-5.patch against head version. Still can't access page arguments in functions.

add1sun’s picture

Thanks for the rerolled patch bonaparte. I looked at it very briefly and basically the problem is that

1) the page_callbacks were not set right:
The drupal_get_form didn't have a form being passed in, just the arg. It needs both. (new patch just does that)

Once I set that correctly, the thing is that the callback (commentmail_deleteban) is NOT a form. The function is being called as one though, hence you are getting an empty form array in.

In addition 2) the callback code for approve and deleteban are all kind of wacky. There are two functions available each for approve and deleteban: a _quick_ version and regular (eg, commentmail_quick_deletban and commentmail_deleteban). Currently neither _quick_ one is being called anywhere. I don't have time to look at this right now but we need to look at the differences between the two, what they are doing and if we need them both. If not, get rid of the one we don't need and clean up the remainder. If we need them both then we need to make sure they are getting called properly in the code. We also have the two respective _submit handlers as well.

So, if someone can sit down and take a look/figure that out (or someone who is familiar with the code and already knows what is supposed to be happening) that would be grand.

add1sun’s picture

I should also point out that an upgrade of this module may be obviated by Actions and Triggers in D6. You can now send an email on comment creation right in core. If actions_tokens can let you get links back to actions, then there is no point in this module AFAICT.

add1sun’s picture

Hm, I briefly looked at this a bit further (the actions/trigger thingy) and basically the one big thing that commentmail offers is the choice between all comments or only ones that need approval. Commentmail is also offering much more info that action_tokens isn't .

sun’s picture

Also, doesn't Comment Mail allow the user to choose whether she likes to be notified?

greggles’s picture

I just tested this out because I wanted to be sure of how similar this module is to what can be accomplished with token_actions.

The main part that can't be accomplished is the set of quick edit features at "comment/qa/CID".

You can get close to this by providing links in the email for:
1) Edit comment
2) Delete comment
3) Ban IP

I'm now testing out token_actions which, at least until today, was at least slightly broken. I'm also working on a "do what commentmail does" recipe for token_actions.

greggles’s picture

Oh yeah, and commentmail does the filtering for whether the comment is published or not which is not possible with core actions/trigger.

Anonymous’s picture

subscribing.

robertDouglass’s picture

Please take the t() out of the defines at the top of the module.

maartenvg’s picture

It's time we decide which road to take with the D6 branch. There are a few options
* A simple and straightforward port to D6, without introducing no new ways to do stuff, thus creating a very clean upgrade-path, but duplication functionality from core.
* Make a recipe using D6 actions & triggers, together with Token Actions. This will limit the usefulness of the module (no filtering on approve/non-approve, no quick-edits) and requires quite an amount of tinkering by the user.
* Do half-port/half-actions&triggers. Introducing the appropriate triggers for filtering and/or the appropriate (configurable) actions, but keeping the quick-edits and limit the amount of configuring. I'm not sure a seamless upgrade can be guaranteed with this way, but of course a site-specific manual can be provided.

Either 1 or 3 are fine by me, with a slight preference for number 3, because using core functionality is a good idea.

robertDouglass’s picture

Start with #1.

physiotek’s picture

subscribing

kbahey’s picture

Status: Needs work » Needs review
FileSize
11.84 KB

The patch no longer applies.

Here is another attempt against the DRUPAL-5 tag.

maartenvg’s picture

Status: Needs review » Needs work

That patch is not correct.
- see #18.
- use of db_last_insert_id() is incorrect
- there are some menu callbacks missing
- the patch removes the quick approval page functionality.
- combining the above 2 issues, it breaks the upgrade path.

I've created a 1-to-1 port already (based on the patches and comments in this issue), but I don't have access to it at the moment. I will issue a alpha/beta/rc release as soon as possible.

maartenvg’s picture

Status: Needs work » Fixed

Sorry for the long wait. I wanted to be sure that the security issue in D5 was fixed before releasing anything for D6, but it was a bit delayed.

But now it is in! Therefore, I just committed a full port to D6 and created a Beta-release (see http://drupal.org/node/339844) for it. Have fun testing and using it.

Status: Fixed » Closed (fixed)

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