Reset event for Flag

Hi,

I have the same problem. And the result is also ok, flags are resetted.

....

Actually, for all reported abuse (node, comments, users), I got this error when reseting flags, also using the option " Link type : Confirmation form ", the confirmation message in the form is wrong, i always something like " Are you sure you want to report this user as offensive? " or the equivalent for comments or nodes.

Thanks for your help,

I have the same problem too

- corrected status (needs work only relates to patches)

- error confirmed (not very helpfull, I know)

- both the messages in the log and the wrong question during resetting the flag

To clarify: The errors are only thrown when I use the admin options at admin/content/flags/abuse_comment (note: tested with comments). Resetting the flag with the link on the comment works without problems.

Same behaviour concerning the display of the wrong "unflag confirmation message". The correct message (at least after changing the text - not sure about the behaviour before) is displayed when I reset directly on the comment but the completly wrong message (as reported it is the "Flag confirmation message") is displayed when I use the admin interface.

my fix - probably temporary

apply for

; Information added by drupal.org packaging script on 2009-05-12
version = "6.x-1.x-dev"
core = "6.x"
project = "flag_abuse"
datestamp = "1242130241"

Could you please submit an actual patch? http://drupal.org/patch

I don't think using a global here is a good indicator, will see if I can duplicate this error.

I think I found a better way of handling this, however it would require a patch to flag module. Would appreciate some reviews here: http://drupal.org/node/571100

Great to see you working on it. I hope I have the time to test these patches tonight.

Patch works. No more ugly messages in the watchdog when resetting a flag from the admin interface. No interference with other flag/flag abuse functions (so far).

The wording needs still some work (I guess this patch shall implement the feature and is not the last word): "Flag confirm" is not really useful, is it?

Also on the comment itself the link reads: "Reset flags (1)".

Thx!

Ah, I see what is happening with the label. Within flag.inc, flag_confirm() uses $flag->get_label() which uses the $action as part of the string for the key identifier of the flag's text. So what it seems we need to do is extend the flag's labels along with our actions. So looking at our flag definition within flag_abuse.flag_default.inc, along with the keys 'flag_confirmation', 'unflag_confirmation', we need a 'reset_confirmation' message. Probably extend the others as well. I'll update the patch and see if flag will pick these up... might need to expand that patch for flag module.

Ok so I've updated the patch for flag module over at #571100: Add "reset" action and hooks to Flag which will allow this next patch to function properly. It was a little more involved to get the default options and text to play nicely and to be able to extend these actions, but I'm pretty happy with the results. Flag module is not getting a lot of love these days, so if nothing happens in the next couple of days I'll ping quicksketch and see what is up. Maybe he could use some help maintaining over there.

Important parts of this patch include:

1) better checking of access to the reset flag and when to show.

2) use $flag->get_label() instead of hardcoded text. This allows the user to change this text in the flag settings to what they want.

3) added count to the reset flag.

4) event checking within our hook_flag implementation.

5) two queries within our hook_flag implementation when resetting flags instead of a flag api call for every single user that has flagged this type.

6) Implementations of hook_flag_TYPE_default_options() for each flag which is of course dependent upon the flag module ticket referenced above.

Sorry to bother you with it but I could need some help applying this patch. I am using cygwin and normaly everything works fine when I put the patch into the folder where it is needed and use the patch < patchfile.patch command there.

Your patch seems to change not only flag_abuse.module but also includes/flag_abuse.flag_default.inc and the way I apply patches no longer works. Cygwin complains about a file it cannot find at input line 132 and suggest using the -p option. After quoting the text where the patch failed cygwin asks for the file to patch but I cannot figure out the correct input.

I am a real noob concerning command line usage and lullabots command line basics hasn't covered this yet. ;) The advise in the drupal.org documentation about applying patches doesn't help me neither.

So a little bit of coaching would be very much appreciated because I really want to test these patches.

I actually have no idea how to use cygwin as I'm on mac. The command I use is:
patch -p0 < flag_abuse-reset.patch

I assume this is what it means when it tells you to use the -p option.

Thx. That did the trick.

I am trying this on a quite fresh local instal (acquia stack) but after successfully applying these patches I get:

warning: include_once(./sites/all/modules/flag/flag.module) [function.include-once]: failed to open stream: Permission denied in C:\Drupal\acquia-drupal\includes\bootstrap.inc on line 611.
warning: include_once() [function.include]: Failed opening './sites/all/modules/flag/flag.module' for inclusion (include_path='.;C:\php5\pear') in C:\Drupal\acquia-drupal\includes\bootstrap.inc on line 611.
warning: include_once(./sites/all/modules/flag_abuse/flag_abuse.module) [function.include-once]: failed to open stream: Permission denied in C:\Drupal\acquia-drupal\includes\bootstrap.inc on line 611.
warning: include_once() [function.include]: Failed opening './sites/all/modules/flag_abuse/flag_abuse.module' for inclusion (include_path='.;C:\php5\pear') in C:\Drupal\acquia-drupal\includes\bootstrap.inc on line 611.

I just downloaded a fresh copy of acquia drupal, flag and flag_abuse and applied the patch for flag module mentioned in this thread, the patch for flag_abuse in this thread, enabled the abuse_node flag (noting that the various reset messages were indeed present) and then generated some nodes with devel_generate and clicked on the "Flag as offensive" link. This had the desired effect taking me to the flag confirmation with the appropriate message and upon confirmation, back to the front page where the link then said "Reset flags(1)". Upon clicking that link I got the expected confirmation again with the appropriate message for the reset event and upon confirming the expected action was again experienced.

@dddave: not sure what you did wrong, but everything seems kosher to me.

Renaming this issue a bit to reflect what is going on. Also, after some feedback from Flag we need to change how we're implementing hook_flag_TYPE_default_options()

Updating patch to reflect the change from hook_flag_TYPE_default_options() to hook_flag_default_options().

Ok since this will be utilizing features that are only to be found within the 2.x branch of Flag module, I'm going to start a new corresponding branch and mark this to be implemented there.

The changes in this patch utilize the new alter that went into Flag 2.x, drupal_alter('flag_options', $options) which I'm trying to get extended #577076: Extend hook_flag_options_alter() so that we can do type detection as illustrated with this patch.

The process for testing this patch is as follows:
Upgrade to Flag 2.x
Apply the patch at #577076: Extend hook_flag_options_alter() to Flag module
Apply the patch at #571100: Add "reset" action and hooks to Flag to Flag module
Upgrade to Flag Abuse 2.x
Apply this patch to Flag Abuse module

Sorry for coming back to this so late. Applying the patch to flag abuse is no problem but preparing flag itself seems to be.

I figured that #577076: Extend hook_flag_options_alter() has been comitted already. Applying the second patch fails for Hunk #1 at line 211.

Any movement to be expected?

Here is an updated patch. However, I'm having trouble getting the alterations to work properly now... @see #623270: hook_flag_options_alter() not working

I had the same problem described in this posts. However I can not figure out watch combination of modules/patches I should be using. Could someone tell me what ver. of flag and flag_abuse I should be using, as well as what patches to either I will need to apply. This would be greatly helpful as I have already spent several hours trying multiple combination unsuccessfully.

This patch should apply to the latest 6.x-2.x-dev as marked in the issue. However, the original approach was pretty much negated by quicksketch in the Flag 2 issue queue as he doesn't think it is a good idea to extend Flag in this way. As such, I've not kept up with the patch there (#571100: Add "reset" action and hooks to Flag) and there have been other changes to Flag that make that patch no apply correctly anymore.

Also in that issue I pretty much cede to quicksketch - but when I tried the other approach, it was a hell of a lot more code and I got discouraged and then simply didn't have the time to pursue it further. It basically seems like a 'simpler' solution is going to add a lot more code to flag_abuse unless we can go back to the drawing board and figure something else out.

Thanks sirkitree, I've also noticed that there is no Flag Abuse 6.x-2.x-dev, unless I am missing it. I'd be happy to test out any new patches your have in the working

It's is not on the release page as it is currently the same as the 1.x-dev but can be checked out from cvs.

So what does this mean? If flag cannot be patched for a fix anymore and flag abuse has not been patched to do things the alternate way, does that mean that using flag-2.x + flag_abuse-2.x.-dev will generate error messages every time abuse flags are reset?

Well, quicksketch has created a patch here: #571100: Add "reset" action and hooks to Flag

This allows the resetting of flags, but it has not been committed because it has been languishing in the issue queue marked as "needs work". I just set it to "needs review". I can also re-work the patches here to use quicksketch's preferred approach, but not until that patch is committed! Catch 22.

I will work a patch up as though it is committed, use it to review and test quicksketch's work and then mark that issue as R&TBC as long as everything works as expected. If you want this issue to be closed, we need to:

1. Get quicksketch's new flag resetting function committed to Flag 6.x-2.x-dev
2. Use it to re-work this patch

If anyone else wants to test/work on this, please start in the Flag queue and review #571100: Add "reset" action and hooks to Flag.

I am very, very interested in testing anything new on this front. Thx for working on this. At the moment I am a bit unsure if I can already help. Is the patch in http://drupal.org/node/571100#comment-2232882 the thing you want feedback of?

@dddave, yes, testing that patch, confirming it works and getting it committed is the first step.

@dddave, yes, testing that patch, confirming it works and getting it committed is the first step. It will allow development to continue here. Until it is available to work with, we cannot use it.

I did this changes on line 70 in flag_abuse.module

      $uid = db_result(db_query("SELECT uid FROM {flag_content} WHERE fid = %d", $flag->fid));
      $flagger = user_load($uid);
      if ($flagger) {
        // Supposed to pass in a full $account here, let's see if we can fake it
        $flag->flag('unflag', $content_id, $flagger, TRUE);
      }

WAS:

      $query = db_query("SELECT uid FROM {flag_content} WHERE fid = %d", $flag->fid);
      while ($result = db_fetch_object($query)) {
        // Supposed to pass in a full $account here, let's see if we can fake it
        $flag->flag('unflag', $content_id, $result, TRUE);
      }

Instead of giving only uid in user object ($result) for $flag->flag function (3rd argument), I'm loading full user object ($flagger) and everything works perfect.

Flag - 6.x-1.3
Flag Abuse - 6.x-1.0

I download the Flag Abuse - 6.x-2.x-dev but still have the reset problem, the patch #21 doesn't seem to work for me. Maybe i manually edit wrongly but anyway I try the #31 and and just replacing below line in flag_abuse.module

$query = db_query("SELECT uid FROM {flag_content} WHERE fid = %d", $flag->fid);
while ($result = db_fetch_object($query)) {
// Supposed to pass in a full $account here, let's see if we can fake it
$flag->flag('unflag', $content_id, $result, TRUE);
}

to

$uid = db_result(db_query("SELECT uid FROM {flag_content} WHERE fid = %d", $flag->fid));
$flagger = user_load($uid);
if ($flagger) {
// Supposed to pass in a full $account here, let's see if we can fake it
$flag->flag('unflag', $content_id, $flagger, TRUE);
}

and the reset work for Flag Abuse - 6.x-2.x-dev with no error in the report.

Request to confirm and add it the cvs. Thank

So, first a thought - the above note worked for me as well...

With that said, I figured I'd take a stab at re-working the patch found in #21 to actually function...

In the process of doing so, I quickly realized that the scale of this 'patch' would become bit comical - at this point I've relocated or rewritten the vast majority of the flag_abuse code...

None the less, I've gotten a decent amount working in my re-imagined version of flag_abuse:
- Added a new flag_link_type 'Confirm + Reset' - with a bit of work, this could be (and probably should be?) its own flag plugin.
- This new link type uses flag_reset_flag to do its work
- Added support for Modal Frame API via use of a patched version of Automodal
- AJAX response when using Modal Frame API for popups

A few caveats which definitely need work:
- link_type is not locked as 'confirm_reset' - it should be, as my re-imagined abuse flags aren't intended to work with other link_types
- locking the link_type removes the conditional options - these need to be added to the abuse flags' option forms

Also, I don't think I've broken anything else when using Popups UI / JavaScript disabled, but I haven't thoroughly tested this
- AJAX response only works with Modal Frame API via a modified version of Automodal.

The functionality produced via Automodal could be included directly based off Modal Frame API, but seeing as how Popups UI has quite a few compatible modules (but is abandoned) I figured I might get further use out of creating a relatively easy way to port modules from Popups UI to Modal Frame API.
Anyhow, to get a usable version of Automodal, from the 6.x-1.x-dev release, the following patches are required:
http://drupal.org/node/767128#comment-2833562 (http://drupal.org/files/issues/automodal_multiple_selectors.patch)
http://drupal.org/node/767128#comment-3129644 (http://drupal.org/files/issues/automodal-hook_automodal-767128-1.patch)
http://drupal.org/node/685364#comment-3129642 (http://drupal.org/files/issues/automodal-message-callback-685364-1.patch)
http://drupal.org/node/767128#comment-3139080 (http://drupal.org/files/issues/automodal-allow_functions-767128-2.patch) - updated

and I'm not sure if this is a jQuery version related problem, but I also needed:
http://drupal.org/node/767128#comment-3129824 (http://drupal.org/files/issues/automodal-multiple_selectors-767128-3.patch)

PS: Local testing seems fine, but my development server is far from a normal or stable environment so... results may vary? :p

NOTE: This patch is a working draft, it is not ready for consideration - It's locally functional, but that's all I can promise, adventurous testers proceed with caution!

Edit: I see spacereactor couldn't get #21 working either. Ok, needs work then - but I don't think the latest patch is a solution. It's just too big. It would be more useful to get #21 working without any re-imagining, trying to live with the things you don't like just to make it work. Re-writes ought to be for D7 at this stage in the game...

Wow, that's a monster!

Does #21 not work? I'm not sure why it wouldn't... and I think it would be better to test and tweak that than starting again and adding so much functionality. I can't see sirkitree ever having time to read and commit this!

Re: #21, the problem sirkitree was having was marked as fixed in Flag, both the dependent patches have been committed to Flag, so versions 6.x-1.3+ will function with the patch. Also the Flag Abuse dev snapshot has not changed since that patch was written, so it should apply perfectly well. And it doesn't kill so many kittens.

LOL - Thanks Greg - yes I was about to reply to this over the weekend after testing, but was just overwhelmed a bit and didn't want to spend time on a module I actually rarely use myself right now. Thank you for recording your work for us though @Vector-! I will not be re-writing this module for 6.x at this point for sure though. As Greg mentioned, a re-write would have to be for D7, working with Flag 7.x-2.x

The patch in #21 has several problems - in fact, it was actually my starting point for producing my patch... but it needs a lot of work, because quicksketch did not commit the dependent patch - he committed his own re-imagined version which is not an event at all, but a hook.

My original intention definitely was not to re-imagine the module - it's simply where I ended up while fixing up #21, based on the patch actually committed to Flags.

The patch is also decidedly not well-formed - it's just a cut of my current working stuff, and I wouldn't expect it to be committed in its state, or even considered.
At the very least, it needs:
- Modal Frame API stuff moved to its own issue
- Flags administration form stuff moved to its own issue
- A considerable portion (but possibly not all?) of the confirm_reset link_type stuff relates to one of the above two

When I get a chance to come back to this, I'll section the changes out into more reasonable pieces for consideration - and resolving the above stuff into separate issues should make this much easier to consider - for now I'm just documenting my work so I can remember what I've done when it comes time to actually prepare it for legitimate contribution :)

(Or if someone cares enough about the AJAX response to actually go through all of the blather above... I'd be interested in hearing about that too ;)

A few notes (mostly to myself) on relevant pieces to resolve:
- hook_flag_options_alter() argument order - I think I also had issues with this working as expected, related to link_type.
- flag_reset_flag() doesn't reset counts (and the DB query for it in #21 is wrong) - need a hook_flag_reset_flag()
- can't set event to reset, need the logic somewhere, probably hook_flag() for reduced changes
- I think the new link_type was my response to // seems there should be a better way to do this... - relevant code should probably move into hook_preprocess_flag()

So this was all a bit much. There is a new function in Flag 2.x called flag_reset_flag which will work for this just fine. Commit it to the 2.x branch soon.

committed