Rules integration for notifications

You're right, that could really cover(or at least simplify) many additional use cases. Thanks for the suggestion, and for already offering to help testing!

Alright since nobody has taken a stab at this yet,

• I have created some rules action that will fetch the list of saved searches that need to run and supply those as a result.
• Based on those saved searches I created another action to fetch the new items for those saved searches.

If someone has a better direction on how the rules integration should be implemented i'm all ears.

I included a screenshot on how I implemented the rule.

Update to the patch, I realized after that my returns were a little off and could generate some warnings.

Thanks a lot for this patch! Looks pretty good already, on the whole.

+  $ids = db_select('search_api_saved_search', 's')
+    ->fields('s', array('id'))
+    ->condition('enabled', 1)
+    ->condition('notify_interval', -1, '=')
+    ->where('last_execute >= last_queued')
+    ->where('last_queued + notify_interval < :time', array(':time' => REQUEST_TIME + 15))
+    ->execute()
+    ->fetchCol();

This is more or less copy-pasted from the cron hook, but you filter for those with notifications explicitly disabled. I expect that's not on purpose? Otherwise: Why?
In any case, we should have that just as a helper function, not duplicate the code. This, along with some other code style improvements, is contained in the attached patch.

The same goes for search_api_saved_searches_rules_get_saved_search_new_items(), which also seems to be largely copy-pasted: please create a new helper function to use in both places in the code (unless not possible for some reason).

Otherwise, this looks pretty good, thanks again!
However, I don't have Rules set up (or much experience with it), so didn't test it. Some feedback from Rules users would be good before committing.

Thank you for the quick reply.

In the admin interface to "disable the notifications" you are required to use a negative value that is the reason I used a -1 for the notification_interval query. So that is why I was searching explicitly for disable saved searches, but maybe there should be a better way to configure that in the admin interface ?

I did go back and remove the copy and paste code and separated out the function for less repetition. I am hoping I didn't break anything in the process but it should be tested just to make sure. I also moved the rules hooks into the .rules.inc file as per the standard rules docs.

The scenario that I am implementing this for is for a user to be able to create messages in the message stack using these rules.

The move to a rules.inc file is a good idea, thanks! However, it makes the interdiffs basically worthless, so let's do that only once we're otherwise finished with and sure about the patch.

Your patch, apart from some trailing whitespace issues, would have broken normal notifications by not loading the result items anymore. I now added that to the normal cron hook code, the old mechanism should now still work as before. Will still need a few people to verify this indeed doesn't break anything, though.

Also, I now see why you'd filter for searches with disabled notifications: if you're using Rules to send notifications (or do whatever), you of course don't want the normal cron-based implementation to interfere.
However, this doesn't really work if you still want to use the notification interval to determine which searches to execute, right? If that's -1 in all cases, you'll just always execute all (enabled) searches, each time your run the rule. For that, a general admin option for disabling cron-based notifications (or some other new solution) seems to be required, I'd say. Otherwise, I can't really see this working properly.

Agreed that using the -1 as interval for a flag to remove the notifications is not ideal.
I added an admin setting to not add the item to the queue if the disable email notifications for the saved searches boolean is checked.

Agreed it would be good to have other people review this and make sure the functionality is not compromised and the rules integration is fairly intuitive.

Thanks, I think that's a good solution for the problem.
However, I think we should keep this consistent with the "Use activation mail for anonymous users" option. Also, there's no reason to hide the "activation mail" options when disabling notifications.
Finally, we have to take care to use isset() or empty() to avoid PHP notices for existing sites (which have no value set for the option yet).

Patch attached which should fix all that. Please test/review!

Patch looks good. The only thing left to do before committing the code would be to move the rules code into .rules.inc file.

Beside that is there any other functionality that might be required ?

Patch looks good. The only thing left to do before committing the code would be to move the rules code into .rules.inc file.

OK, great to hear. Thanks again for all your work on this!
Committed (with *.rules.inc move).

Beside that is there any other functionality that might be required ?

If so, people can just open new issues for that.