Closed (fixed)
Project:
Rules
Version:
8.x-3.x-dev
Component:
Rules Core
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
15 Jan 2018 at 08:21 UTC
Updated:
8 Mar 2018 at 16:29 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
jonathan1055 commentedHere's a patch which adds the new 'default revision' flag for core 8.5 and above. Previously we only needed to change the text, so the inline conditionals were simple. Now, we need to conditionally add two new elements to the array, but also keep them in the required order. As we don't add keys to the expected array and rely on the automatic numeric numbering I could not simply add the new values at the end in a new statement. So I used array_merge and an inline conditional. It works, but looks a bit ugly and might not be very readable. To make the patch simple to understand I have not indented the main set of array items for now. This can be done in a later patch when/if we have agreement that this is how we want to solve the problem.
Also, automated testing is currently broken for Rules at 8.5 and up - see #2936553: Rules kernel tests fail with "Creating default object from empty value". Unfortunately one of the failing tests is the Autocomplete test, so the change in this patch will not actually get checked at the moment. But I have tested it locally and it fixes the problem (which I only discovered during local testing when working on that other issue.) When the core issue is fixed this patch can be retested.
Comment #3
jonathan1055 commentedHere's the patch corrected to pass PHPCS coding standards.
Comment #4
fagoSounds good, but let's postpone this until we see that it does not break testcoverage!
-> Postponed on #2936553: Rules kernel tests fail with "Creating default object from empty value".
Comment #5
jonathan1055 commentedYes, we need to hold off on this as the core change has actually been reverted out from 8.5.x and currently only exists in 8.6.x - see #123 on #2891215-123: Add a way to track whether a revision was default when originally created
Comment #6
jonathan1055 commentedThe change has now been put back into core 8.5 - see #2891215-135: Add a way to track whether a revision was default when originally created. So the patch in #3 above should work exactly as before, and fix the tests for 8.5+. Re-queuing to check this.
Comment #7
jonathan1055 commentedThe tests now pass at 8.6 and 8.5 with this patch, and 8.4 still passes as expected.
Would be nice to commit this, then we will have all green test passes for all three core versions.
Comment #9
fagothx, committed.
Comment #10
fago