Port the Rules integration to D7

Have anybody verified that our Rules integration works with D7's Rules? No point in fixing the link unless it does.

Rules2 has changed considerably in D7, I'm not very up to date with it, but I doubt it will "simply" work.

I am running latest release of D7, Flag, and Rules and also have this problem. Rules integration broken, no option to choose "after node/comment has been flagged" for triggering a rule.

The overview of changes can be found here: http://drupal.org/node/906482

Attached is a first start on porting the integration, it's note complete yet (nor tested). Rules 2.x currently misses proper support for identifiable data types being non-entities. I'll work on fixing that.

after this functionality, shout if you need any testing I can at least lend a hand there.

Subscribe...

Reclassifying as "task". It's not really a bug.

@fago:

- Will old saved rules continue to work?

- If you think #405316: Rules: Use flag as a 'regular' argument has merit, feel free to tackle it here, if you want to.

Subscribing, critical stuff for my project :)

>- Will old saved rules continue to work?
Will have provide some simple settings-converters for that if necessary. However the upgrade path is one of the missing parts that keeps rules 2 from going into beta phase.

>- If you think #405316: Rules: Use flag as a 'regular' argument has merit, feel free to tackle it here, if you want to.
Yep, that's the only way to do it in Rules2. However, we won't need the loading action as users can just switch the parameter configuration mode. So we need to provide the input form + specify flags as parameter.

I try to use flag with rules for notifications. It should work with Drupal 6 but I would use Drupal 7. So I have to wait and look at this issue ;) I hope it will work together in the near future :) Thanks

subscribing

Subscribing.

subscribing

When it happens? There are any news on this question?

I'm waiting for rules integration. I won't press somebody to give us a date but I very interested to know if the integration progress goes on?

@JohnnyX: The Rules module has changed quite much between D6 and D7, so there's a risk that porting the Rules integration will take some time.

It is possible that the work has begun already, but if you want to be sure you could send a message to one of the maintainers (or look in the code of the dev version). If you contact a maintainer, I bet that they would be happy if you volonteered to start the work. :-)

Cheers,

I've just added support for identifiable, savable non-entity data types in Rules. While I think modules should better implement the entity API instead, I see that this requires a bit more than just having load() + extractId() as core also defines presave(), save(), .. hooks one should invoke then too. Given that, it's probably a good thing to be able to work with such non-entity data-types in Rules too, nevertheless.

Anyway, based on that improvements I've ported the flag integration. In my basic tests everything worked fine.
I converted the form to make use of the new tableselect form element + moved the include file in the root directory, as unfortunately the d7 hook_hook_info() 'group' feature doesn't support lazy-loading includes from an 'include' folder.

Patch attached. Requires the latest Rules version.

-> I've just tagged Rules-2.0-ALPHA-5, which you can use to test this.

Awesome. Looking forward to testing.
Thanks for your work.

any testers? :)

Flags are available but I haven't work with rules before. So a have no working example...

I would use it for notification emails (updates or new comments for flagged content) but have problems to create the rules/ filters.

Subscribing... I will have a stab at testing this sometime soon, hopefully.

Wow! Not only does it work – there's a whole lot of new fun stuff added! (I especially like the "node is flagged" condition.) Respect @fago.

I have not tested all the triggers, conditions and actions yet, but everything I've tried works great. Count on me starting to use this.

I guess there should be more testers before this is RTBC, but you could also argue that adding this patch right now would make Flag and Rules together much better than it does currently – even if there are bugs left.

Great work!
//Johan Falk

PS: For anyone else who wants to apply the patch, run it against Flag beta5 and not dev and you'll have an easier time. You'll probably get two warnings, but these concern changes that are already made to the Flag module – no need to panick.

Will or already is the patch commited to flag module?

@JohnnyX: It seems parts of the patch is included in the Flag module, but the most is not.

Will or already is the patch commited to flag module?

I won't be touching Flag (and probably Drupal) in the foreseeable future. Sorry. I notified Nate of this two weeks ago. Somebody should step into the co-maintainer (or maintainer?) role.

q0rban, maybe? He's been submitting quite a few D7 flag patches lately that it'd be nice to have committed...

q0rban, maybe? He's been submitting quite a few D7 flag patches lately

I've noticed only one patch. Perhaps I'm mistaken. But this ought to be handled by Nate (the maintainer), not me. Anybody: feel free to suggest yourself and/or prod others to action (and remember that you can email Nate privately).

Uh. Weird. I think I imagined it. Maybe I was looking at the same patch over and over >_>

I re-rolled the patch to apply again. Due to the Git migration CVS $Id$ got stripped, what let the patch fail.

Perhaps we should just get it in and fix any possible issues with it in follow-ups?

Tested the patch a little.
Seems to work very well indeed!

I've not tested this a great deal but it certainly seems stable enough and I'd be inclined to agree with #33 that any issues that do exist could be dealt with later.

I've done some quick tests (created some flags and some basic rules) and the patch seems working :-)

sub scribe

Hello,

I've done some tests on the patch in #20, and everything works great, except for anonymous users.

I have a flag (not global) on a node that marks it as "viewed"
and I was trying to create a rule to check if the node is already flag before flagging it. In the condition under the "USER ON WHOSE BEHALF TO CHECK" I inserted site:current-user, this works great for logged in users, but when the condition is run with an anonymous user I get an error.

The error in my case is in this function:

function flag_rules_condition_flagged($flag, $entity, $account) {
return $flag->is_flagged($flag->get_content_id($entity), $account->uid);
}

because $account is empty.

in my scenario I realized I could remove the rule and still have everything work like I want it (flaggs a node again even if it's flaged).

But there is a problem here.

Should site:current-user provide the anonymous session info? or should there be a different rule with a different selector and then we would add a condition to check if the user is logged in first.

Thanks

Idan

Hello,

I've begun to get this error:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '2-15' for key 'PRIMARY': INSERT INTO {flag_counts} (fid, content_type, content_id, count) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3); Array ( [:db_insert_placeholder_0] => 2 [:db_insert_placeholder_1] => node [:db_insert_placeholder_2] => 15 [:db_insert_placeholder_3] => 2 ) in flag_flag->_update_count() (line 743 of ..sites/default/modules/flag/flag.inc

here's how I have it set up.

1. flag called "viewed" per user (not global).
2. a rule with the event " node is being viewed" and an action "flag node" - under flag I chose "viewed". node: node . user on behalf to flag - site:current-user. Permission check is unchecked.

could this be related to the fact that i'm flagging a node each time it is being viewed even if it is already flagged?

another question, site:current-user seems to work when flagging - so why doesn't it work when checking if the node is flagged?

Idan

subscribing

Nice to see an exemple of how to implement RulesDataUI btw.

+        'flagged_' . $flag->content_type => array(

If we combine a 'A [node] has been flagged, under "@flag-title"' event with other events providing a "node" variable, this will be a problem, since provided variables are not the same ("node" and "flagged_node". Why not use the entity name directly to avoid this kind of situation ?

Any chance to release a new dev/ beta version of flag includes the last patch?
No flag maintainer available?

subscribing

subscribing

subscribing

subscribing

ad #41: Indeed, however this would require more work on the upgrade-path later on, as it would have to care about the changed variable name compared to drupal 6.

@fago: Ah, makes sense, did not think about that. However, this feels like a big limitation to me, it narrows the use of the rules flag integration. If we agree about that, we definitely should have an upgrade path instead.

Subscribe. I'd like to test the patch in #33, but is there any other effort that can be put into getting this into a -dev release (or stable?). This is the one and only issue that's holding back my use of D7 for a couple projects.

(Update: Patch in #33 working great for me. No problems to report.).

The problem is a lack of maintainers (with time) at the moment.
mooffie is gone now and quicksketch is too busy.

In addition to that, I'm also rather opposed to Rules module to begin with, but that's due to my developer-nature I believe. I'd honestly like Rules support removed and maintained separately from Flag module.

I would be completely behind the idea of a separate "Flag Rules" module, if that's a way to finish off this issue and resolve any problems here. I love the Rules module for quick site feature builds... I don't use it for everything, but it's often easy to build an entire workflow on a Drupal site using Flag + Rules.

I'm not really bothered where the functionality ends up, so long as we have it.

So... Any takers?

I tested the patch in #33 to some length yesterday, and it worked well for almost all cases. However, if I try to use Rules to set a flag, I get an error message:

"EntityMetaDataException: Invalid data value given. Be sure it matches the required data type and format. in RulesIdentifiableDataWrapper->set() (line 642 of … … /rules/includes/rules.state.inc)."

This error was given when trying to flag a newly created node, on behalf of the node author – but my guess is that it would be the same for other events and flag types.

@quicksketch: I don't agree with you about the Rules module. But I guess that's due to my site-builder and configuration nature.

Hey @Itangalo: Yeah I know Rules is very popular and a lot of people don't build a site without it. Unfortunately I'm not someone that ever uses it, making it so that trying to get me to review Rules patches is always a painful experience. Mooffie a big supporter also and I really respect his opinion on such things, which is how Rules integration got in to begin with. Unfortunately Mooffie has bowed out of Drupal development for an indeterminate amount of time, so there aren't any maintainers left to review these patches, hence why I think it would be best served by someone that uses both Flag and Rules in a dedicated project.

@quicksketch: Thanks for the explanation – I understand and respect it.
(I do, however, think that you're really missing something: check out how to set node access with Flag + Rules + Content Access if you feel like arguing about this.)

If the problem is reviewing patches, I hope I can find someone to help (eventhough I can't review myself).

Itangalo is right that there are significant benefits to using rules.
I use it all the time - and I've used it a number of times with flag in D6, to great effect.

But as I said in #53 it doesn't really matter that much if its in a separate module.

Plus, I would be curious if quicksketch (or a future co-maintainer) would consider merging a separate flag rules module back into flag as a sub-module later on.

I have no real preference for either option, other than for whichever one will get flag rules integration into a proper release (whether it be a release of flag or a release of a separate module) first.

I did some more testing of the patch today, and it seems that the Rules condition "Comment has flagging count" freaks out. I get an error message like this when setting a flag triggering this condition:

An HTTP error 500 occurred.
http://…flag/flag/comment_spam/26?destination=node/23&token=_JduoENdR48VlC3RCQdOkz_7W6rEbankGQHIvvB0FX4

The error is only triggered when the flag count is above the set limit in Rules, and it doesn't seem to depend on what kind of action will be performed. (Also, the action isn't performed – indicating that the condition isn't returning TRUE.)

Sorry for bad news.

@55: I see and fully understand your reasoning. However, I'd prefer having the Rules integration directly in the flag module, because:

* The Rules integration is not much more than the flag.rules.inc file, which gets automatically included whenever its needed by Rules. So the rest of the module or users don't using Rules would not suffer from it.
* Having a separate module for each Rules integration is overkill and unnecessarily scatters the module page.
* Also, having this directly in the module, makes this *great* functionality much more visible to users. Also from a UX point of view "just go and use it", instead of "install yet another module" is superior.

Thus, I'd love to keep it here. As I'm going to use it anyway, I'd volunteer for maintaining the Rules integration directly in flag.

ad #58: Yep, we need to fix that and also go for #41. Then, this needs some tests. But let's figure out where it goes first.

Probably no surprise to anyone, but I agree with fago's reasoning in #59. The strongest argument is, I think, his third.

Having the Rules integration directly in the Flag module is as natural as having its Views integration – nobody would like to install a separate module for that. The only option, I think, is having the integration in the Rules module instead.

Since I consider both Rules and Flag essential modules, it is difficult for me to say "who should provide support for who". If looking at usage statistics though, Rules is more than twice the size of Flag – which suggests that Flag should be the module hosting the Rules integration (and not the other way around). If there are personal preferences for another solution I don't mind at all, as long as the integration doesn't require any extra modules.

By the way: Flag is an awesome module, too. Thanks for all the work you (quicksketch) put into it.

Subscribe

Subscribe, please port. Great module, but without Rules integration is useless for me :(

subscribe

I hope rules integration would be ported. Flag is a great module but without rules integration it loose much of it's power and options...

This looks very useful, subscribing.

Subscribe

I tried testing this, but upon applying the patch I get "fatal: git diff header lacks filename information when removing 1 leading pathname components (line 5)"

Subscribe - this is a must to upgrade any of my sites to D7. Please let us know if any help is required.

Also can anyone say if the Flag 6.x-2.x-dev suffers from the same issue? I know this is not the place but there is no discussion anywhere else. Thank you.

UPDATE: Flag 6.x-2.x-dev still works well with rules.

I use patched flag module and I create a test rule.
The rule set a value to a field is content is flagged or unflagged. Text example works fine!

Is it possible to do some more tests and commit the patch (I patched it some time ago and have to searching for the patch file at my local pc...) or create an additional module with the functionality?

Subscribing

I think it should be the last patch from fago...

Just bumping a bit…

@fago: Have you had any chance to reiview the bug reports in #54 and #58?
(Sorry I haven't tried to solve them myself.)

I re-rolled the patch against current master branch on git, since it wasn't applying properly on my end.

I've implemented the port and it works fine except for I'm having trouble with the tokens. I am trying to send a system mail message once content is flagged. For some reason the tokens for any cck text fields are returning blank values within the email. Any reason why that could be happening? It will send other flagging user information such as email address, url, and even a date widget but not text input.

sub

subscribe

subscribe

+1 for rules integration within flag module. A separate module for flag/rules integration seems overkill.

Use case: Using drupal commerce to create an order where users can flag nodes. Within the order are node reference fields and a "maximum" number of nodes that can be flagged (varies by order, thus node reference has unlimited number of fields). When user flags node, rules checks the order flag limit and allows/disallows the node to be flagged.

I have a developer background, but believe "if you don't need to create a custom module, you shouldn't" This allows future site maintainers that may not be developers to instigate changes.

Applied patch in #33 successfully.

I there, I'm sorry, I'm really struggling to apply the patch at #33 or #73 [I always get some error: can't find file to patch at input line 5].
Could someone please share is "patched version" of Flag Dev ? It would be really appreciated [not sure it's the right place to right this...]

@tbeauchemin: Attached.

ATTENTION: It seems that something is wrong with the attached, patched Flag version. Don't use it. Download the patch and Flag dev version, and patch it.

The command for applying a patch is "git apply [name of patch file]". Patches are applied from the base folder of the project being patched.

after i create my rule and click on a flag i got that error

EntityMetadataWrapperException: Invalid data value given. Be sure it matches the required data type and format. in RulesIdentifiableDataWrapper->set() (line 649 of /home/stefan/public_html/drupal_version/sites7/uba/modules/rules/includes/rules.state.inc).

if i use js to toggle the flag i got an 500 http error

used the code from post #80

anyone else have that problem?

E: without rules and flag actions i have no problems.

@Itangalo - i just tried your patched flag version with latest version of rules - unfortunatly i get the error:
( ! ) Fatal error: Class 'FlagRulesDataWrapper' not found in /Users/steffen/test/sites/all/modules/rules/rules.module on line 356

The FlagRulesDataWrapper Class is availabe in flag.rules.inc ..

Thx for help

I was able to get the patch at #73 working, thanks!

At first I kept getting HTTP 500 errors when flagging users and activating a rule, but clearing the cache twice did the trick.

sub

Tried #73 and #80 - I get the following error from both

Recoverable fatal error: Object of class stdClass could not be converted to string in eval() (line 1 of \sites\all\modules\rules\modules\php.eval.inc(146) : eval()'d code).

Rules config:
Event -> A user has been flagged, under "Flag-Name"
Condition -> user has role -> Parameter: User: [flagging-user], Roles: role 1
Action -> empty (because I was testing to find the problem)

will Flag be available as a condition as well, e.g. as in
- on EVENT abc
- CONDITION: content/user is flagged
- ACTION: xyz

sub +1 for the integration

Subscribe

Subscribe... Get the error from #81, #54. Tried all patches

+1

Subscribe

+1

I've fixed the integration to work with the latest Rule code (see #54) and put it in a sandbox, see http://drupal.org/sandbox/fago/1256260

Also I've added an action for getting the users who flagged something. Updated patch attached also.

Patch #93 worked for me, before I was having the same error as #54 when setting the rules action "Flag a node", and after applying the patch, the node is being flagged automatically with no errors.

Thanks fago!

Patch #93 working here also.

Thanks

I've been testing fago's patch in #93 and it's working flawlessly. I've set up several different Rules configurations using flags as both events and actions and I've had no errors whatsoever.

The only issue I've run into is allowing anonymous users to flag content via rules (using the "Flag a node" action). I've got Session API module installed and anonymous user flagging works fine if I'm not using the Rules integration. But because the only data selector available to me for the "Users on Whose Behalf to Flag" option is "site:current-user", there doesn't appear to be a way to set a flag for an anonymous user (who by definition isn't logged in).

Fago, is there any way the Rules integration could somehow provide a data selector for both the session ID of an anonymous user and a logged in user?

Thanks,
Ben

The patch doesn't cover session-id based flaggings of anonymous users. This really is a separate issue we had not in d6 either and could be added in a follow-up.

Thanks, fago, for the clarification. I'll post the anonymous user stuff as a separate follow-up issue.

Since we have at least 3 reports of fago's patch working very well and no new bugs reported, I'm marking this as RTBC. If I've jumped the gun on this, feel free to change the status back. But I'd love to see this patch committed soon. :-)

--Ben

please port to dev, it's difficult to apply patches on many files on windows.

I'm hoping the anonymous user aspect can be covered as well (in a follow-up). It's the last piece of functionality I need out of Flag to get my site operational.

I've added fago as a maintainer to the Flag project, so this can be his first commit. :)

fago will now officially be maintaining the Rules integration within the Flag project. I've also added fubhy as a maintainer to help clean up the issue queue and pull out old integrations that are preventing the stable 2.0 release of Flag.

Awesome, and welcome to flag, Fago!

@quicksketch: Great, thanks! :)

Committed.

Subscribing.

This just brought down our site.

Either Flag needs a dependency on Rules, or we can't autoload FlagRulesDataWrapper as it extends a class that doesn't exist if Rules is not enabled.

I confirm that the rules are not shown anymore in "React on event".

BenK, I see that you want to create an issue for anonymous users.
But I already have an error with latest patched version (not with a latest dev of 27 Aug)
if logged as anonymous user I got:
Notice: Trying to get property of non-object in flag_rules_condition_flagged() (line 399 of Z:\home\paris9.kz\www\emploi\sites\all\modules\flag\flag.rules.inc).

I used:"site:current-user"

Is it the same issue you have meant or another one?

ouch.
It seems odd that conditional includes are not supported by the class registry. Attached patch implements an ugly work-a-round, please test.

Update: Please do not use this issue for any troubles with *using* the Rules integration. Instead, create separate issues for that.

ah, I figured out that the registry was not the cause of the breakage, but an old left-over in hook_theme(). Removing that does it.

Hello,

After applying patch #109 and #110 i get the following warnings:
<-----
Warning: include_once(/home/liberpro/public_html/sites/all/modules/flag/flag.rules.inc) [function.include-once]: failed to open stream: No such file or directory in _theme_process_registry() (line 413 of /home/liberpro/public_html/includes/theme.inc).
Warning: include_once() [function.include]: Failed opening '/home/liberpro/public_html/sites/all/modules/flag/flag.rules.inc' for inclusion (include_path='.:/usr/share/pear:/usr/share/php') in _theme_process_registry() (line 413 of /home/liberpro/public_html/includes/theme.inc).
Warning: include_once(/home/liberpro/public_html/sites/all/modules/flag/flag.rules.inc) [function.include-once]: failed to open stream: No such file or directory in _theme_process_registry() (line 413 of /home/liberpro/public_html/includes/theme.inc).
Warning: include_once() [function.include]: Failed opening '/home/liberpro/public_html/sites/all/modules/flag/flag.rules.inc' for inclusion (include_path='.:/usr/share/pear:/usr/share/php') in _theme_process_registry() (line 413 of /home/liberpro/public_html/includes/theme.inc).
----->

Also for me the patch is not working.

Let's not get sensational in the issue title. We haven't even made a "release" including this functionality yet.

I've committed #110 which should prevent flag.rules.inc from being loaded if Rules doesn't exist.

+1 for the awesomeness of this. Thanks for great work to all involved.

Thanks quicksketch!