Closed (fixed)
Project:
Rules
Version:
8.x-3.x-dev
Component:
Rules Core
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
4 Jan 2020 at 00:53 UTC
Updated:
3 Mar 2020 at 17:04 UTC
Jump to comment: Most recent, Most recent file
Minus a few features that aren't fully implemented. Still, a big improvement in usability.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | 3104328-14-enable-disable.patch | 3.02 KB | tr |
| #11 | 3104328-11-list-builders-no-enable-disable.patch | 21.48 KB | tr |
| #10 | 3104328-10-list-builders-no-enable-disable.patch | 20.98 KB | tr |
| #8 | 3104328-8-list-builders.patch | 29.97 KB | tr |
| #6 | 3104328-6-list-builders.patch | 27 KB | tr |
Comments
Comment #2
tr commentedAdded missing file, also added a few test cases from D8RE for the enable/disable functionality.
Comment #3
tr commentedI forgot to include the changes D8RE made to RuleExpression, which is where we check to see if a Rules is disabled before executing it - this is why the test failed. I will work on that tomorrow.
Comment #4
tr commentedThis checks for disabled Rules in EventComponentResolver rather than in RuleExpression, which I think is better ... Let's see if the testbot likes it ...
Comment #5
tr commentedHmm, same two fails ... That implies I'm still missing something from D8RE, because these same tests work in D8RE ...
Comment #6
tr commentedOK, let's go back to using the changes to RuleExpression that have been working in D8RE, and hopefully that works.
Then we can re-visit using EventComponentResolver in another issue.
Comment #7
tr commentedWell the good news is that patch fixed the previous fails.
But the bad news is, changing RuleExpression by adding an additional constructor parameter means that a whole bunch of Unit tests broke. I'm going to have to go back and fix those tests to account for the new parameter...
Comment #8
tr commentedThis should fix a bunch, but not all ...
Comment #9
tr commentedI'll get back to this in a few days ...
Comment #10
tr commentedThe part causing the problems is the enable/disable functionality. There seems to be a problem with the way Rules handles caching.
So I'm going to commit this in two parts. With this first part I'm leaving out the button to enable/disable Reaction Rules. The rest of the enable/disable code is in there, and you will still be able to enable/disable using drush, but you will not have access to this functionality from the UI.
This first patch implements the changes made by D8RE to the Reaction Rules and Rules Component list builders, which provide the overview page. This is a huge improvement in the Rules UI, especially with the Ajax filtering, so I don't want to leave it out much longer because it is impeding other work on the UI.
Comment #11
tr commentedComment #13
tr commentedCommitted #11. Leaving open to finish porting the enable/disable part.
Comment #14
tr commentedHere's the missing part of the patch, which just adds the enable/disable buttons to the Reaction Rules list builder and adds a few tests of the enable/disable functionality. All the functional code was committed previously in #11, only the buttons and the tests were left out.
This should be working now that #3108494: Exception when updating cache with empty data was fixed - it was a caching problem that was causing the enable/disable tests to fail.
Comment #16
tr commentedCommitted.
Comment #17
jonathan1055 commentedThanks for this, such a useful feature.
Good that we solved the cache problem(s).
Comment #18
CraigBertrand commentedYou guys rock! Thanks!!!
Comment #19
jonathan1055 commentedCheers Craig. It's great when someone recognises the effort.