Could you please consider upgrading the module to 5.0?
Thanks
jacauc

CommentFileSizeAuthor
#25 comment_notify_5.patch18.57 KBgreggles

Comments

inozz’s picture

+1

almarma’s picture

+2

Thanks

Christoph C. Cemper’s picture

working on it ... a friend already helped with it...
still first got a couple of 4.7.x bugs/issues to solve that will then be merged into a 5.x version

filiptc’s picture

Hey Chris, what's the status? I have no more nails to bite... :)

fhelmschrott’s picture

yeah a 5.0 Version would be great indeed ;)

patchak’s picture

Any news on this module for 5.0 ??

This is a pretty useful btw, i'd love to use it on my new blog!

Thanks,
Patchak

funana’s picture

subscribing.
You know that I love this module and think that it's a must... Now that I want to switch to drupal 5.1 with a lot of sites, what's the status Christoph?

Christoph C. Cemper’s picture

we are already working on the update for 5.1 - ETA is Jun 15

funana’s picture

great news, Christoph! Thank you very much for your work!

JBI’s picture

+1

greggles’s picture

subscribing.

Also, if you can post any existing patches here I could help review and work on them. thanks!

patchak’s picture

I pyed Christoph 50$ to get the patch done for 5.1. It was supposed to be ready for june 15th, still no news tho... Maybe Christoph could post an update here??

Christoph C. Cemper’s picture

Hi there and sorry for the delay once again

can you read minds ? just 2 hours ago I started testing the alpha version here :-)

I will post a 5.x kit by the weekend for interested beta-testers to help out

best,christoph

PS: will any of you guys come to Barcelona?

greggles’s picture

great news (and yes, I'm going to be in spain for a month including Drupalcon).

funana’s picture

As I said before, I will test it. Just send me a mail, you got my address ;-)

Christoph C. Cemper’s picture

Hey guys, I'm still testing this and the drupal_mail function is giving me headaches... for some reason the way mails are sent out in 5.x is WAY different than it used to be

I get werid headers and crazy html tags withing the notification emails where I didn't have any in the past version

any good doc you can point me to for migration ?

greggles’s picture

Cristoph - please post a patch and a specific description of your issue here. Debugging telepathically doesn't work very well.

Christoph C. Cemper’s picture

already found that, the mail function gets the mail headers another way :-)

Christoph C. Cemper’s picture

beta version will be out for test tomorrow

Christoph C. Cemper’s picture

Hello folks,

Sorry for all those delays, as I promised the port a couple of times already.

Here is the beta version of comment_notify for your testing.

I already run in productive on http://weblog.cemper.com

best
christoph

Christoph C. Cemper’s picture

I am not allowed to upload the zip to drupal.org anymore, just tar's - and in fact i'm too lame to create a tar file WITHOUT all those CVS control files (control files of my local cvs excluded) ... dunno why drupal org disallows zip - I think it SUCKS big time!

So I posted the zip here

http://weblog.cemper.com/a/200707/04-comment-notify-for-drupal-5x-beta-o...

I'll setup the CVS version in Drupal.org after the beta test went fine

christoph

greggles’s picture

cristoph - please make a patch file. That is the proper way to attach files to an issue. zip files are blocked because they are less useful than a patch file.

See http://drupal.org/patch and http://drupal.org/handbook/cvs for more details about how these can be created.

In short: cvs diff -up > patchfile.patch

Christoph C. Cemper’s picture

Greggles, sorry I won't create 4 patch files just to allow somebody test my beta.
(this is a new VERSION, not a quick hack!)

If you got a problem downloading a zip to help beta-testing,
please please just wait until I put everthing in CVS and created the 5.x version.

Christoph C. Cemper’s picture

and the reason is plain simple - not only to I have extra work with creating diffs, then the next day I get support requests from people wanting to help testing, but unable to apply patches...

not everybody capable of finding bugs is a unix crack.

greggles’s picture

Status: Active » Needs review
StatusFileSize
new18.57 KB

here are the changes from the zip as one patch file.

cristoph - can you clarify the comment.module file that is in your zip file?

It looks like you added a - comment_invoke_comment($edit, 'publish'); when the comment status is not COMMENT_NOT_PUBLISHED.

I apologize for not understanding this already - can you clarify why that's necessary as opposed to just using the 'insert' op?

Is this something that should get fixed in core?

funana’s picture

Thank you very much Christoph!

Posted a reply on your site, found a missing ',' in line 349 of patched comment.module, fixed it and it works well!

Christoph C. Cemper’s picture

@greggles:

inserting comments ( as they are are when a spam bot hits your site ) is a BAD time to send out mails... only when moderation automatic/human has published the comment we may send notifications, else this will get a mail blacklist war

yes, I spoke to killes & co last year about this small change required and posted it as request here
http://drupal.org/node/156475 again...

A LOT of users asked for this already,
and as you can see the patch you created patched comment.module wrong /once again/
(see funana's comment)

however, your help for creating the patch and general support/advice is much appreciated - please help me push this mandatory but small change into core ASAP

thanks
christoph

Christoph C. Cemper’s picture

I forgot to add, that you need to run the update.php to migrate the mail text ...

I'm confused why the comment.module didnt work for you,
as I took the exact same version as it's running on this site.
I thought you took the .patch created by greggles on drupal.org

Thanks again for testing,
and please chime in HERE

http://drupal.org/node/156475

with a +1 so we soon can get rid of patching comment.module

Christoph C. Cemper’s picture

Version: 4.7.x-1.x-dev » 5.x-1.x-dev

after a month of beta tests the 5.x version has been added to CVS and the release script should give a downloadable tar in a couple hours

best
christoph

Christoph C. Cemper’s picture

Assigned: Unassigned » Christoph C. Cemper
Status: Needs review » Closed (fixed)