Port Rules integration for Scheduler to Drupal 8

I try to unpublish an entity by scheduling, but it doesn't work.
Is this the reason?

I have the next error-report.

[Thu Feb 25 22:06:45.507905 2016] [:error] [pid 4537] [client 192.168.2.8:50640] PHP Fatal error: Call to undefined function rules_invoke_event() in /var/www/drupal8/modules/scheduler/scheduler.module on line 386, referer: http://192.168.2.10/node/29/edit?destination=/admin/content

Systeemnaam: scheduler
Versie: 8.x-1.x-dev
Vereist: ActionsDatetimeFieldNodeTextFilterUserSystemViews

Systeemnaam: rules
Versie: 8.x-3.0-unstable7+43-dev

problem solved, after deinstall rules module.

Theo

Hi tzsl,
Yes, you are right that the cause of your error is that we have not converted the Scheduler rules functions to 8.x yet. I had not yet installed Rules 8.x on my development site, but I have now and get the the same errors.

In the interim before we finish the porting, it is a problem that Rules cannot be installed at the same time as Scheduler. I will make a patch which temporarily avoids these fatal errors, so that at least you can still use Rules with other modules, whilst Scheduler dev is also installed.

Thank you for bringing this to our attention.

Jonathan

Having read #2245015: [META] Rules 8.x Roadmap and http://d8rules.org/ and got the documentation from https://thefubhy.gitbooks.io/rules/content/ I started to convert our scheduler.rules.info into the 8.x files.

However, I am having trouble testing my changes because I think the actions and conditions UI is not fully working. There are no data selection drop-downs so you have to guess what to enter for some fields and it is not clear what is required. Incorrect data can be saved, eg entering a string instead of a node interface, which causes a permanent site crash. The only way I could get out of it was hack the code in the rules module to ignore the parameter. This was on a test site without Scheduler installed.

I am going to leave this work, as the Rules code does not seem stable enough yet.

jonathan, if you update to the latest dev of Rules 8.x-3.x you will find data selection capabilities added to the UI. looking forward hearing if that helps

Thanks dasjo, I have just downloaded the latest dev and it is much easier to use. I can now get on with converting our actions and conditions. It was worth me adding a link on #2470627: [Meta] Rules 8.x integrations for contrib modules

Here's a start to the rules conversion. There are no test coverage yet, so it won't prove anything, but just sharing progress so far.

I have now done more of the conversion of our rules to 8.x. Progress so far:

Conditions

Four conditions as per 7.x now converted into .php files in src/Plugin/Condition/
These all seems to be working correctly under manual testing.

Actions

Four conditions converted into .php files in src/Plugin/RulesAction/
These are working but still have problems:
1. The date value passed is defined as an integer. There should be a way to define it as a date, so that a pop-up calendar can be used when picking a fixed date for the rule. Also need a way to pass in a date from a node - currently I have just managed to set it by hard-coding a number.
2. The number/date value is used when updating the node, but is not stored permanently. So we have a problem with the entity cache being out of sync with the field data table. Needs more work.

Events

Six events converted into .php files in src/Event/ and scheduler.rules.events.yml
Four of the events are invoked when saving a node via hook_node_insert or hook_node_save (in scheduler.module)
The other two events are invoked when Scheduler operates on nodes during cron (in scheduler.cron.inc).
These seem to be working but still have problems:
3. When a node is published during cron the evenst for 'node is updated' are also triggered. This can cause nesting and a crash, because the node update process appears to call itself. More investigation needed!
4. In 7.x we passed the two scheduler dates as 2nd and 3rd parameters, but I could not get this work for 8.x. The dates are available in the node object for the 'on save' events, but for the cron events the actual node date values have already been removed, hence the need to save the values and pass them as parameters to the action

Here is a patch for all the work so far. I am also going to create a new branch to commit this. The patches are big, and it will be easier to share and show work-in-progress with a separate branch.

Here is the first commit to a new branch called 8.x-rules
http://cgit.drupalcode.org/scheduler/commit/?id=4187e55
I was hoping that an auto-commit comment would be added to this issue, but it has not appeared yet, hence adding the commit link above, for info.

I've just had a look at the code in the commit you referenced in #12. I see that you created new Events, but this will conflict with what we've implemented in #2669164: Introduce event subscriber for the Scheduler hooks ?

What is the relationship between events created for Rules and events defined by Scheduler for use in the events dispatcher? I know they will be triggered by the same actual thing happening in real life, but can they be defined as the same thing within the module? I mean, can we write it in such a way that we use and reuse exactly the same coding and files for both purposes?

I have merged the main 8.x updates into the 8.x-rules branch, to allow more testing.
https://www.drupal.org/commitlog/commit/142/9a65c4bb2748da307915128bd3ef...
https://www.drupal.org/commitlog/commit/142/ab9728bbdd0ce1f13545a60acf2a...

The Rules module still has plenty of things which have not been converted yet, and some usability problems with basic functionality like data comparison against the node title, but one of my requests #2702127: Active / Inactive toggle for rules does have a patch, and that has been helpful.

Merged 8.x-1.x commits into 8.x-rules
http://cgit.drupalcode.org/scheduler/commit/?id=5449463

Merged latest 8.x-1.x into Rules branch
http://cgit.drupalcode.org/scheduler/commit/?id=dcdd1f9

I decided that the rules progress so far is good enough to go into the main code branch. I have merged all Rules commits from the 8.x-Rules branch and will probably delete that branch in due course. All rules work will now continue in the main 8.x-1.x branch.

In SetPublishingDate and SetUnpublishingDate change date parameters from @ContextDefinition("integer", to @ContextDefinition("timestamp", and remove redundant ->save()

First simpletest file for rules - checks the four conditions (two for node type enabled and two for node is scheduled)

fail: [Other] Line 35 of modules/scheduler/src/Tests/SchedulerTestBase.php:
Unable to install modules rules, node, scheduler due to missing modules rules.

Maybe we need

test_dependencies:
  - rules

in the .info.yml file?

I have raised #2775519: Automated testing for 3rd-party modules integrating with Rules to ask for ideas on why the Rules module is not available when running tests on d.o.

FYI, we are running the dev branch of scheduler, and after doing a recent composer update, some ajax requests started returning:

Fatal error: Class 'Drupal\rules\Core\RulesConditionBase' not found in /Users/mike/Sites/puppet/webroot/modules/contrib/scheduler/src/Plugin/Condition/NodeIsScheduledForPublishing.php on line 23

Installing the rules module solved the problem, but if rules is going to be a dependency, it should be listed as one.

Hi mikemccaffrey,

I have created a separate issue to deal with this:
#2790459: Move Rules integration code into a sub-module for accurate dependencies

Jonathan

@mikemccaffrey I have created a patch to fix the dependency problem. See #5 on #2790459-5: Move Rules integration code into a sub-module for accurate dependencies. It would be great if you have the means to test it.

I now have tests for all conditions, actions and events that Scheduler provides for Rules.
Adding 'rules' as a main dependency in the scheduler.info.yml is only a temporary test, to see if that module gets loaded and available for use in the tests. Listing it under 'test_dependencies' should have been enough, according to https://www.drupal.org/node/2000204 but it is worth a try to see what happens using 'dependencies'.

I just had another idea - maybe the dependent modules are scanned and installed and made available for use in the tests before the patch is applied and tested. If that is the case then in requires a commit of the .info.yml dependency before any Rules tests will pass. i.e. it is not possible to add dependencies via a patch and have those dependent modules available for use in testing that same patch.

I might make a temporary commit to add Rules as a dependency, then remove it when I commit #2790459: Move Rules integration code into a sub-module for accurate dependencies

This patch is the same as #31 but with no change to .info.yml as that is now done in the commit above

OK, so the test failures are not linked with the Rules patch above - I have reversed out the temporary commits above and the codebase is still failing https://www.drupal.org/node/3292/qa
It seems our tests pass at 8.1 but fail now at 8.2 and 8.3 - they were working at the previous commit on 25 August.

I have created a new issue to track this:
#2799095: Fix schema error in scheduled content view. Status value should be string not boolean

That's good. Following my idea in #33 above now that the committed .info.yml contains
test_dependencies: - rules the Rules module is loaded at test start-up as required, and we can actually get test results for the Rules integrations.

The failures in the patch from #35 are due to D.O. simpletest loading the recommended Rules release, which is 8.x-3.0-alpha1 from 7th March. There has been quite a bit of Rules development since then, and some major faults have been corrected. On my localhost I get these test failures with the alpha1 release, but with alpha1+12 from 8th May and with the latest dev alpha1+26 from 26th August all the tests pass.

I have committed the changes in #2790459: Move Rules integration code into a sub-module for accurate dependencies - the details are in http://cgit.drupalcode.org/scheduler/commit/?id=30ece5e

During my testing I realised that neither Core nor Rules provide rules actions for setting a node to be published or unpublished. These are needed for our Rules testing and for general use in Scheduler-based rules, hence we need to provide provide these actions, until such times as they are available in Core or Rules. Patch attached for two new files in the new sub-module

New patch for the three new test files. Do not need to include 'node' in the static $modules as this is already a pre-requisite for the main module. Likewise, do not need 'rules' in the tests module list for scheduler_rules_integration as this is already a pre-requisite for the sub module.

These tests will still fail for the same reason as in #43 but there should be fewer failures now that the new actions for publish and unpublish immediately have been committed in #45. We need to either (a) work out how D.O. testing can be forced to load the latest dev release of Rules, or (b) encourage the Rules maintainers to release alpha2

Hi Jonathan,

I'm trying to apply the patch from #46 against 8.x-1.x but I'm getting some errors;

git apply -v 2651348-46.rules_full_test.patch                                                      
Checking patch src/Tests/SchedulerTestBase.php...
error: while searching for:
  /**
   * The modules to be loaded for these tests.
   */
  public static $modules = ['node', 'scheduler'];

  /**
   * A user with administration rights.

error: patch failed: src/Tests/SchedulerTestBase.php:19

Should I use a different branch?

Hi Levi,
Ah, sorry about that. No, you are using the correct branch. Those changes to src/Tests/SchedulerTestBase.php have already been made and committed, as I needed that additional permission in the new tests for #2700209: Disable node access checks during cron publish/unpublish. Just delete the whole section of the patch on SchedulerTestBase.php and leave in the majority, which creates the three new files for rule testing.

I am quite pleased with the tests. I had to do quite a bit of researching and trial & error on how to test Rules. I don't think many other contrib modules have done it yet. Beware, though, that you need the latest dev release of Rules. It fails if you run the alpha1 release because the Rules folk have made major fixes. That's why I cannot commit these tests, as D.O. simpletest only uses the latest tagged release, not the -dev release. Unless you know how to alter that behavior?

Based upon #49 I've created a new patch so that we can continue looking into this issue.

Now with code within the patch..

Setting to needs review, to run the tests and check that it still fails on D.O.

The Rules module development seems to be happening on GitHub mainly now. I wonder if they have plans to release alpha2, so that other contrib modules can actually start to use Rules and work on their integrations?

[edit: our posts overlapped, I did not intend to remove your patch, but when I re-posted it seems my patch file had been uploaded anyway. It does not matter, as they were identical].

I have raised #2807765: Release 8.x-3.0-alpha2 for Rules to try to push for a new tagged release, so that the D.O. tests will pass.

Well, I'm going for an approach now where we introduce a composer.json file within the scheduler_rules_integration module. Maybe this will allow us to use a different branch.

Rules have now tagged an alpha2 release, so now lets try the rules tests again.

That's nice. I will commit these tests now.

Levi, good idea about adding a .json file to the sub-module. You can still do that, as I presume it will be useful anyway, even if it is not possible to specify a latest dev release for the dependencies.