Comments should be able to be flagged as well.

(P.S. This module is fantastic - Drupal was seriously in need of this)

CommentFileSizeAuthor
#17 flag_content_anything_4.patch15.96 KBAnonymous (not verified)
#15 flag_content_anything_3.patch15.47 KBAnonymous (not verified)
#14 flag_content_anything_2.patch15.17 KBAnonymous (not verified)
#12 flag_content_anything.patch15.08 KBAnonymous (not verified)
#5 flagcontent_comments.sql.txt108 bytesadiatis
#4 flagcontent_comments.patch6.99 KBadiatis

Comments

kbahey’s picture

kbahey commented

This is already in the wishlist in the README file.

Come on folk ... patches welcome ...

patchak’s picture

patchak commented

This seems to be a good way to moderate comments in 4.7 Any chance you would have the time to build something like this??

Just by curiosity, how long could it take to build that function??

Thanks

kbahey’s picture

kbahey commented

It is not a priority item for me, and I have no time for it.

If some client wants it done, and they pay for it, it may get done.

If someone else submits a patch, then I will include it as well.

adiatis’s picture

adiatis
Primary language Dutch
Location Belgium
commented
StatusFileSize
new flagcontent_comments.patch6.99 KB

A patch to allow comments to be flagged. Please give feedback if it is ok.

thx

adiatis’s picture

adiatis
Primary language Dutch
Location Belgium
commented
StatusFileSize
new flagcontent_comments.sql.txt108 bytes

I forgot to give the mysql changes

There is already a field 'flagmsg'. This will be used in the near future to allow the users to give a reason.

kbahey’s picture

kbahey commented
Status: Active » Needs work

Thank you for this patch.

The way it was done would indeed work, but the design is not future proof (cid in the same row as nid, but I can't think of a better solution right now).

I would have put this as "patch needs review" so others can comment, but you need to address the points below.

- make the patch in unidiff format. Follow the instructions here: http://drupal.org/patch
- follow the coding style detailed here http://drupal.org/node/318
- database change has to go in the .install file, as an _update_N() function so users can upgrade.

Thanks again for sharing this.

zigma’s picture

zigma commented

adiatis,

I tested your patch. Two things that i noticed-
1) There could be more than one entry in flag_content table for any node. Possible when one user flags two comments under the same node. nid is primary key. So it throws uniqueness exception.

2) I see the column for message while flagging a content. Not sure if you implemented that as well. I did not get any interface for writing the message.

thanks to kbahey and adiatis for this long awaited module.

neclimdul’s picture

neclimdul
He/Him
commented

This is something that interests me though I'm not able to work with the patch given here. We'll see where this goes.

kbahey’s picture

kbahey commented

I looked at the patch that is attached here, and another offline patch.

Neither of them have the ideal data model that I am looking for.

neclimdul and myself discussed a good data model yesterday, but neither of us has the time to spare for this at the moment.

jonathan_hunt’s picture

jonathan_hunt commented

Can you outline the solution you are thinking of? I would like to have this functionality on Drupal 5.x, and I might be able to put some time towards it.

Anonymous’s picture

Anonymous (not verified) commented
Title: Allow comments to be flagged » Allow Anything to be Flagged

this should be like votingapi where you can pass a cid / type but cid should be an eid (entry id).

If you are doing it for comments, mind as well make it so anything can be flagged.

Users for example, flag a user for putting something offensive on their profile?

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new flag_content_anything.patch15.08 KB

I got someone asking me to do this for users so I whipped up a version that is ready to expand for anything.

Currently I setup some switch()'s in the various spots needed and altered the table a bit to work for 'types'.

This is good for now and fully functions well.. i think though that someone may want to look at a hook system for handling types.. votingapi needs this as well.

I fixed a few other issues I noticed in this module as well and also added support for drupal_get_destination().

The admin email also now supports path.module as before it had url('/node/1') and url does not want the beginning / .

After running the patch, run update.php so you get the flag_content table all up to date.

changes to the table..

<?php
  $ret[] = update_sql("ALTER TABLE {flag_content} CHANGE nid eid INT( 11 ) NOT NULL");
  $ret[] = update_sql("ALTER TABLE {flag_content} ADD type VARCHAR( 25 ) NOT NULL AFTER eid");
  $ret[] = update_sql("ALTER TABLE {flag_content} DROP PRIMARY KEY");
  $ret[] = update_sql("ALTER TABLE {flag_content} ADD PRIMARY KEY ( fid )");
  $ret[] = update_sql("ALTER TABLE {flag_content} ADD INDEX ( eid , type , timestamp )");
?>
Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review

oh by the way.. cvs checkout was a DRUPAL 4.7

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new flag_content_anything_2.patch15.17 KB

missed 1 table alter in update_1()

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new flag_content_anything_3.patch15.47 KB

missed 1 alter and tweaked a few other things. also was a bug in hook_link()

kbahey’s picture

kbahey commented
Status: Needs review » Needs work

Steve,

Thanks for this. I think this is the right direction to take, and more inline with my thinking.

I saw at least two implementations, one of them the original from this issue, and another one offsite. Neither of them has a good data model. Yours does have a decent data model.

You also need to update the .install file to have the new table structure in case someone is installing the module for the first time.

After you do that, I will give this patch a more indepth review.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review
StatusFileSize
new flag_content_anything_4.patch15.96 KB

here ya go.

pgsql not done so would be great if you could maybe handle that.

may want to think about developing a hook for handling the different types instead of the switch statements in the various functions.

someting like.. 

<?php
function flag_content_type() {
return array('type' => 'comment', 'table' => 'comments');
}
?>

or something.

kbahey’s picture

kbahey commented
Status: Needs review » Needs work

Thanks again Steve

Overall, it is in line with the data model that I had in mind.

Questions:

- In the db table, eid is the entity ID. What is fid? Is it just a unique row identifier? If so, it should be an auto_increment field, not a db_next_id (inefficient).

- Comment still has "TODO". Do you plan to work on that?

- Do you have a patch for 5.x?

- Why do you have this $base_url = 'http://'. $_SERVER['HTTP_HOST'];?

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review

hey there.

- In the db table, eid is the entity ID. What is fid? Is it just a unique row identifier? If so, it should be an auto_increment field, not a db_next_id (inefficient).
Actually the drupal way is db_next_id. I took it directly from node.module. This is actually a much better way for more compatibility.

- Comment still has "TODO". Do you plan to work on that?
I left the TODO comments there for someone else to support comments (this should be like a 40 minute task)> A client of mine wanted the ability to flag users so I was funded to do that part.

- Do you have a patch for 5.x?
No I do not sorry.

- Why do you have this $base_url = 'http://'. $_SERVER['HTTP_HOST'];?
This is because you had a global variable which I was told before not to use and it wasn't even working to give proper links in my 4.7 install. $_SERVER vars are already there and this is a perfect place to use this.

kbahey’s picture

kbahey commented

- In the db table, eid is the entity ID. What is fid? Is it just a unique row identifier? If so, it should be an auto_increment field, not a db_next_id (inefficient).

Actually the drupal way is db_next_id. I took it directly from node.module. This is actually a much better way for more compatibility.

Auto increment is much cleaner since it pushes this complexity to the db layer, where it should belong. Drupal core may be moving away from the way it handles next id now anyways.

- Comment still has "TODO". Do you plan to work on that?
I left the TODO comments there for someone else to support comments (this should be like a 40 minute task)> A client of mine wanted the ability to flag users so I was funded to do that part.

OK. Thanks.

- Why do you have this $base_url = 'http://'. $_SERVER['HTTP_HOST'];?
This is because you had a global variable which I was told before not to use and it wasn't even working to give proper links in my 4.7 install. $_SERVER vars are already there and this is a perfect place to use this.

Actually what is missing is using base_path() in conjunction with $base_url as a global, so it is like this:

global $base_url;
...
$blah = $base_url . base_path() . url('user/something', ...);

There is also the issue of backward compatibility. You add the new columns, but existing data should be updated to have the type as node

Anyway, thank you very much for sharing the code. I am working on this to remedy the above and add comments functionality too. I may not have the time to port it to 5.x, but someone else can do that hopefully.

kbahey’s picture

kbahey commented
Status: Needs review » Patch (to be ported)

This is now in 4.7.

Need to be ported to 5.x.

kbahey’s picture

kbahey commented
Version: 4.7.x-1.x-dev » 5.x-2.0
Status: Patch (to be ported) » Fixed
moshe weitzman’s picture

moshe weitzman commented

FYI, drupal core is not moving away from sequences table. the upcoming oracle DB layer needs sequences, and so does sqlite. contrib modules should be using db_next_id() to stay in step with core. sad, but true.

thanks all for this patch. just what i needed.

kbahey’s picture

kbahey commented

Moshe

Oracle has ways of implementing auto increment using sequences and triggers, or INSERT with nextval.

See

http://webxadmin.free.fr/article/oracle-autoincrement-columns-134.php
http://www.databaseanswers.org/sql_scripts/ora_sequence.htm

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)