Comments

TR created an issue. See original summary.

tr’s picture

StatusFileSize
new22.82 KB

Added missing file, also added a few test cases from D8RE for the enable/disable functionality.

tr’s picture

Status: Needs review » Needs work

I 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.

tr’s picture

Status: Needs work » Needs review
StatusFileSize
new23.81 KB

This checks for disabled Rules in EventComponentResolver rather than in RuleExpression, which I think is better ... Let's see if the testbot likes it ...

tr’s picture

Hmm, same two fails ... That implies I'm still missing something from D8RE, because these same tests work in D8RE ...

tr’s picture

StatusFileSize
new27 KB

OK, 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.

tr’s picture

Well 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...

tr’s picture

StatusFileSize
new29.97 KB

This should fix a bunch, but not all ...

tr’s picture

Status: Needs review » Needs work

I'll get back to this in a few days ...

tr’s picture

Status: Needs work » Needs review
StatusFileSize
new20.98 KB

The 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.

tr’s picture

  • TR committed 35e54f5 on 8.x-3.x
    Issue #3104328 by TR: Move list builders from D8RE into Rules - part 1
    
tr’s picture

Committed #11. Leaving open to finish porting the enable/disable part.

tr’s picture

StatusFileSize
new3.02 KB

Here'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.

  • TR committed 7f383b4 on 8.x-3.x
    Issue #3104328 by TR: Move list builders from D8RE into Rules
    
tr’s picture

Status: Needs review » Fixed

Committed.

jonathan1055’s picture

Thanks for this, such a useful feature.
Good that we solved the cache problem(s).

CraigBertrand’s picture

You guys rock! Thanks!!!

jonathan1055’s picture

Cheers Craig. It's great when someone recognises the effort.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.