[drupal-devel] [bug] Consistent return values of comment post(), and much more

[moshe weitzman]

Issue status update for http://drupal.org/node/18656

 Project:      Drupal
 Version:      cvs
 Component:    comment.module
 Category:     bug reports
 Priority:     normal
 Assigned to:  moshe weitzman
 Reported by:  moshe weitzman
 Updated by:   moshe weitzman
 Status:       patch
 Attachment:   http://drupal.org/files/issues/comment_post_1.patch (15.67 KB)

I didn't get any responses so I went and updated it myself ... please do
some testing before committing. This module has many different options
(anonymous posting, placement of comment form, etc.). It is difficult
to test them all (though I did try).




moshe weitzman



Previous comments:
------------------------------------------------------------------------

March 9, 2005 - 14:34 : moshe weitzman

Attachment: http://drupal.org/files/issues/comment_post.patch (15.33 KB)

This function which actually posts comments is comment_post(). This
function is currently pretty messy, and not suitable for use by other
modules. This patch assures that this function returns the comment ID
if the submission was successful, or else returns FALSE. This is
consistent with node_save(). In fact, I've renamed this function
comment_save() for consistency. The old comment_save() was merged with
comment_post(). I renamed comment_validate_form() to comment_validate()
for consistency with node as well.


To do this, I had to clean up quite more of the module. This patch
unifies the admin comment form and the usual comment form, similar to
what we did for nodes. This resulted lots of duplicate code removal.


I was able give Admins the ability to change the author and timestamp
of a comment. Thats been a feature request of mine since Jan 2003:
http://drupal.org/node/983


I removed comment links from the bottom of the comment preview box (as
we did for nodes)


I hope this is considered for commit early in the 4.7 cycle.




------------------------------------------------------------------------

March 9, 2005 - 18:14 : killes at www.drop.org

Much needed patch! I wouldn't mind seeing it in 4.6.




------------------------------------------------------------------------

March 10, 2005 - 15:41 : moshe weitzman

Attachment: http://drupal.org/files/issues/comment_post_0.patch (15.73 KB)

This version properly handles anonymous comments when using a setting
that allows for Name, Email, Homepage on a comment.


IMO, this patch refactors too much code to be considered for 4.6.




------------------------------------------------------------------------

March 12, 2005 - 11:29 : stefan nagtegaal

I tested this patch on current 4.6 RC I and this is, codewise and thus
usabilitywise a must have... Moshe you did a very good job in your
biggest patch of these two..


Dries, I think you should really have a close look at this patch! Its
very, very good and I couldn't find any errors in the code...




------------------------------------------------------------------------

March 18, 2005 - 06:36 : chx

a big +1 for this in 4.6




------------------------------------------------------------------------

March 20, 2005 - 17:50 : Junyor

Attachment: http://drupal.org/files/issues/comment_11.patch (16.2 KB)

This change definitely makes a lot of sense and is overdue.  A couple
comments on the patch:


1) The fix from http://drupal.org/node/17877 is lost (and probably some
other things since this patch was made)
2) There *might* be some missing else clauses in comment_preview for
UID/name checking, but I'm not really sure
3) There are some existing checks in comment_save for name and time. 
Shouldn't those be moved to comment_validate or removed?
4) If you're previewing a comment, there are links for the comment
being previewed, such as edit and reply.  Trying those links throws an
error.  This is an existing bug before the patch.


I've updated the patch to HEAD and fixed a comment typo and an
indentation problem.  I also fixed a typo in the access check in
comment_save.  I hope it isn't bad ettiquette to update others'
patches.




------------------------------------------------------------------------

March 20, 2005 - 18:25 : moshe weitzman

thanks Junyor. Everyone is welcome to update and improve this patch
without consulting me first.




------------------------------------------------------------------------

May 17, 2005 - 23:26 : moshe weitzman

is anyone able to take over maintainership of this patch. i just needs
to be updated for current HEAD. I am unable to do this for a while, and
I fear the patch will just rot in the meatime. Dries and others have
expressed interest, so your efforts will not go to waste.




------------------------------------------------------------------------

May 18, 2005 - 00:04 : Dries

This patch addresses a long-standing issue and will be committed to HEAD
as soon the patch has been updated.